pass or collect comments for ssh private keys #9019
Conversation
|
@alex could you plz review this pr? |
|
It's on my TODO list for this weekend.
…On Fri, Jun 9, 2023 at 12:53 PM James Z.M. Gao ***@***.***> wrote:
@alex <https://github.com/alex> could you plz review this pr?
—
Reply to this email directly, view it on GitHub
<#9019 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAAGBFDMIQJMTC3WV5KRYTXKNIJ3ANCNFSM6AAAAAAYZH4S6U>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
All that is necessary for evil to succeed is for good people to do nothing.
|
| comment_collector: typing.Union[ | ||
| typing.Callable[[bytes], None], None | ||
| ] = None, |
There was a problem hiding this comment.
This API is pretty out of step with anything we have elsewhere. Are there any ways we can avoid a callback like this?
There was a problem hiding this comment.
another way is to save the comments in a runtime attribute of the loaded private key
| if hasattr(encryption_algorithm, "_comment"): | ||
| _comment_attr = getattr(encryption_algorithm, "_comment") | ||
| if isinstance(_comment_attr, bytes): | ||
| comment = _comment_attr |
There was a problem hiding this comment.
I'm not sure what this is for, or where the comment attribute is supposed to come from?
There was a problem hiding this comment.
ok, we can omit checking isinstance of bytes
|
IMO, this signature is too foreign to the rest of our API surface. Another (possibly bad) idea: should we have a separate loader |
|
@reaperhulk another api is ok, but most codes of the new api have to be copied directly from the original one |
close #8980