Skip to content

Comments

Configure message buffer (#82)#92

Closed
mehaase wants to merge 2 commits intoadd_timeoutsfrom
configure_message_buffer
Closed

Configure message buffer (#82)#92
mehaase wants to merge 2 commits intoadd_timeoutsfrom
configure_message_buffer

Conversation

@mehaase
Copy link
Contributor

@mehaase mehaase commented Nov 16, 2018

Add arguments for configuring message queue size and maximum message size.

The closing code is needlessly complex and made the message size code very difficult to write, so I opened up #90 to refactor the way that closing is handled.

The only other thing that stands out to me is that the connection APIs have a lot of repeated arguments now. I know I could use **kwargs to make this more DRY, but then I think (?) that the docs would show kwargs instead of the actual argument names, and that doesn't seem very user-friendly. Thoughts?

@mehaase mehaase requested a review from belm0 November 16, 2018 21:43
@coveralls
Copy link

coveralls commented Nov 16, 2018

Pull Request Test Coverage Report for Build 103

  • 28 of 28 (100.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.3%) to 92.822%

Files with Coverage Reduction New Missed Lines %
trio_websocket/_impl.py 1 93.03%
Totals Coverage Status
Change from base Build 97: 0.3%
Covered Lines: 375
Relevant Lines: 404

💛 - Coveralls

@mehaase mehaase force-pushed the configure_message_buffer branch from f877c39 to 4520d2f Compare November 16, 2018 21:45
@mehaase mehaase closed this Nov 17, 2018
@mehaase
Copy link
Contributor Author

mehaase commented Nov 17, 2018

Umm, not sure how this got closed. Obviously I intended to merge it!

@mehaase
Copy link
Contributor Author

mehaase commented Nov 17, 2018

Merged directly into master.

@mehaase mehaase deleted the configure_message_buffer branch November 17, 2018 14:28
@mehaase mehaase mentioned this pull request Nov 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants