New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
onConnecting implementation #1170
Merged
oberstet
merged 18 commits into
crossbario:master
from
meejah:ticket1158-websocket-headers
Apr 19, 2019
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
e098567
first-cut of an onConnecting implementation
meejah 1528cf1
add TransportDetails
meejah ff6d9f8
more explicit
meejah 6dd5889
cleanups
meejah aa25513
mocks
meejah 28697c4
test fixes
meejah ed251a0
clarify in docs
meejah 11a0f91
pass all possible options from Factory
meejah fde823a
tests for onConnecting
meejah 6361c3e
flake8
meejah d03bd5b
host, port, resource are optional
meejah 58aa219
asyncio transport_channel_id() and TransportDetails fixes from review
meejah a240713
only need ISSLTransport for twisted
meejah 8dbe3fa
use fakes, not mocks
meejah f5d6a6f
extend example
meejah 22c8ad0
flake8
meejah 60744d8
documentation
meejah 368353a
changelog entry
meejah File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are all non-subclassing
IWebSocketChannel
implementations broken now, because they lack this method? Or is the only supported use through subclassing the ABC?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, people that implemented their own sub-classing stuff have broken classes now until they add the new callback (which is strictly expected by the calling library code). not sure if anyone has done that (wrote own classes for channel). adding dynamic attribute checking to work around that .. not sure if it is worth.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, but also the only way to "implement" an ABC is by subclassing it, isn't it? So then they'll get this empty method (which returns
None
and thus "asks for defaults"). I should double-check that, but I'm fairly certain that's how ABCs work...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hrm, I think I'm wrong here ... "A class containing at least one method declared with this decorator that hasn't been overridden yet cannot be instantiated. Such methods may be called from the overriding method in the subclass (using super or direct invocation)."
That's from PEP3119. So, I think what that means is that yes, anyone who created their own classes from scratch and declared the
IWebSocketChannel
"interface" via subclassing will now be broken (because they haven't implementedonConnecting
). A quick test in the repl confirms this -- you'll getTypeError: Can't instantiate abstract class Foo with abstract methods onClose, onConnect, onConnecting, onMessage, onOpen, onPing, onPong, sendClose, sendMessage, sendPing, sendPong
.