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

avoid reliance on pydantic symbols #172

Merged
merged 7 commits into from
Sep 29, 2023

Conversation

jpaniagualaconich
Copy link
Contributor

Fixes #171 and #170

@CLAassistant
Copy link

CLAassistant commented Sep 1, 2023

CLA assistant check
All committers have signed the CLA.

@MasterKale
Copy link
Collaborator

MasterKale commented Sep 1, 2023

mypy is what's failing CI:

Run python -m mypy webauthn
webauthn/helpers/parse_registration_credential_json.py:9: error: "Type[RegistrationCredential]" has no attribute "model_validate_json"  [attr-defined]
webauthn/helpers/parse_authentication_credential_json.py:9: error: "Type[AuthenticationCredential]" has no attribute "model_validate_json"  [attr-defined]
webauthn/helpers/options_to_json.py:20: error: Item "PublicKeyCredentialCreationOptions" of "Union[PublicKeyCredentialCreationOptions, PublicKeyCredentialRequestOptions]" has no attribute "model_dump_json"  [union-attr]
webauthn/helpers/options_to_json.py:20: error: Item "PublicKeyCredentialRequestOptions" of "Union[PublicKeyCredentialCreationOptions, PublicKeyCredentialRequestOptions]" has no attribute "model_dump_json"  [union-attr]
webauthn/registration/formats/android_safetynet.py:91: error: "Type[SafetyNetJWSHeader]" has no attribute "model_validate_json"  [attr-defined]
webauthn/registration/formats/android_safetynet.py:92: error: "Type[SafetyNetJWSPayload]" has no attribute "model_validate_json"  [attr-defined]
webauthn/registration/verify_registration_response.py:206: error: "AttestationStatement" has no attribute "model_fields_set"  [attr-defined]
Found 7 errors in 5 files (checked 48 source files)
Error: Process completed with exit code 1.

https://github.com/duo-labs/py_webauthn/actions/runs/6052677468/job/16427182539?pr=172

You can run this locally to see what errors it's raising. When mypy reports nothing locally it should all pass in CI too:

python -m mypy webauthn

As in #166 fixing some of these probably involves adding # type: ignore[attr-defined] here and there.

@MasterKale
Copy link
Collaborator

@jpaniagualaconich Are you able to see the CI pipeline errors if you click Details above?

@jpaniagualaconich
Copy link
Contributor Author

@MasterKale mypy goes belly up when I install pydantic v1

py_webauthn/.venv/lib/python3.11/site-packages/pydantic/env_settings.py:22: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
If this issue continues with mypy master, please report a bug at https://github.com/python/mypy/issues
version: 1.4.1

I think latest commit will finally make mypy happy but I can't run it myself.

@MasterKale
Copy link
Collaborator

This is looking all but ready to go @jpaniagualaconich, thank you again for taking the time to prepare this PR. Just had one final question before I accept.

@MasterKale
Copy link
Collaborator

Hey @jpaniagualaconich, I know it's been a few days but I do intend to merge this after I get back from TPAC and my schedule clears up a bit to take your changes out for a spin. Thanks for your patience!

@MasterKale
Copy link
Collaborator

Alright, this looks good to me. Thank you for your contribution @jpaniagualaconich 🎉

@MasterKale MasterKale merged commit 6ac0f9d into duo-labs:master Sep 29, 2023
8 checks passed
@MasterKale MasterKale added this to the v1.11.0 milestone Sep 29, 2023
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.

Forced dependency on pydantic for library users
3 participants