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
9713 conch types #12142
9713 conch types #12142
Conversation
…nown-hosts handling
(and fix __all__ to be multiline)
Thanks Glyph for the PR. I will be traveling in the next day... but I will try to get this review. Thanks again. |
Thanks Adi! I just had a fun time accidentally causing my machine to kernel panic with a test bug :) but this should be reviewable now... |
please review |
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.
Thanks for the changes.
Maybe I got the changes wrong.
I am not sure the changes from bytes
IO to str
IO are ok.
There is no misisng code coverage ... and I was expecting the test to fail.
Please see the inline comments related to mode="wb"
vs mode="b"
Other than that, this looks good.
Thanks again.
|
||
@return: an IKnownHostEntry representing the hostname and key in the | ||
input line. | ||
input line. | ||
|
||
@rtype: L{PlainEntry} |
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.
Minor comment. Maybe this docstring can be removed as pydoctor should be able to extract this from the method signature.
I see the rtype
was removed in other methods.
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.
OK, I will get rid of it.
@@ -598,20 +601,19 @@ def body(ignored): | |||
answer = f.readline().strip().lower() | |||
if answer == b"yes": | |||
return True | |||
elif answer == b"no": | |||
elif answer in {b"no", b""}: |
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.
Maybe update the docstring
-'yes' and L{False} when the user answers 'no'. It may errback if
-there were any I/O errors.
+'yes' and L{False} when the user answers 'no' or no answer.
+It may errback if there were any I/O errors.
Discard writes because we are emulating a console object, where they'd | ||
go to the screen, not back into the buffer to be read like an a+ file. | ||
""" | ||
return 0 |
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.
can we assert that we write str or bytes ?
I thins that ByteIO will reject str
for a write operation, but it looks _WriteDiscarder will accept anything.
If this is on purpose, I think that this needs to mentioned in the docstring of _WriteDiscarder
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.
All good. Thanks for the explanations. Much appreciated.
Quick approval appreciated as well! |
Scope and purpose
Fixes #12141
Fixes #9713
This fixes an interactive traceback with Conch, but also adds a bunch of type annotations and testing problems to ensure that other related issues are easier to fix.