Skip to content
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

Fix CI by fixing the type errors for PyNaCl #484

Merged
merged 8 commits into from Jan 19, 2022

Conversation

reivilibre
Copy link
Contributor

No description provided.

@reivilibre reivilibre marked this pull request as ready for review January 17, 2022 17:43
@reivilibre reivilibre requested a review from a team as a code owner January 17, 2022 17:43
@@ -46,7 +53,7 @@ def parse_config(self, cfg: "ConfigParser") -> bool:
print("INFO: Updating signing key format: brace yourselves")

self.signing_key = nacl.signing.SigningKey(
signing_key_str, encoder=nacl.encoding.HexEncoder
signing_key_str.encode(), encoder=nacl.encoding.HexEncoder
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this ought to be encode("ascii") so it doesn't fail silently if we give it something exotic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds fair enough. I don't think it makes sense to put full-fat Unicode into your key.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I found this encoder=... stuff to be fiddley when I was annotating this. But AFAICS they're all meant to take bytes and return bytes. Besides, I think pynacl might want to get rid of them anyway: pyca/pynacl#504 (comment)

@DMRobertson
Copy link
Contributor

Needs a quick linter script, I think. But mypy is happy, thanks!

Copy link
Contributor

@DMRobertson DMRobertson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for this!

@reivilibre reivilibre merged commit df975a6 into main Jan 19, 2022
@reivilibre reivilibre deleted the rei/ci_fail_nacl_signingkey branch January 19, 2022 09:21
@DMRobertson DMRobertson linked an issue Jan 20, 2022 that may be closed by this pull request
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.

Pull in type hints for pynacl
2 participants