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

RFC: Removing Pydantic as a dependency #196

Closed
MasterKale opened this issue Jan 5, 2024 · 3 comments · Fixed by #195
Closed

RFC: Removing Pydantic as a dependency #196

MasterKale opened this issue Jan 5, 2024 · 3 comments · Fixed by #195

Comments

@MasterKale
Copy link
Collaborator

Trying to support the Pydantic v1/v2 migration has sucked all of the oxygen out of the room in this project for me. Pydantic was initially introduced into this project through its use in code that I wrote at Duo which eventually formed the basis of the webauthn==1.0.0 rewrite. However since then it's led to a ton of codebase complexity in an effort to decide how best to migrate the project from Pydantic v1 to v2.

I also need to rethink some of this package's API to expose less of the internal workings. Issues like #173 and #191 highlight how the use of Pydantic is exposed too often, and is part of the reason why the migration from v1 to v2 has been so difficult.

Pydantic is otherwise a phenomenal library, but for the purposes of py_webauthn I think it's time to move away it.

(And come to think of it organizations that vendor all dependencies should welcome this move too as it means four less dependencies to have to update any time a new version of py_webauthn is released... 🤔)

Anyway I'm going to leave this issue open for any feedback on the idea. #195 is passing all tests so I'm inclined to move forward with this, but want to give an opportunity for anyone who might be paying attention time to chime in before I pull the trigger.

@jwag956
Copy link

jwag956 commented Jan 6, 2024

+1 from me (obviously). When you believe #195 is ready I will happily try it out.

@MasterKale
Copy link
Collaborator Author

+1 from me (obviously). When you believe #195 is ready I will happily try it out.

Thanks @jwag956, I appreciate it. If you're okay installing from a branch off GitHub then the code in #195 is ready as far as I'm concerned. I have ideas for other tweaks to follow this but I'll wait till I merge #195. The core package functionality should be preserved and work with minimal refactor needed for existing implementations.

@jwag956
Copy link

jwag956 commented Jan 6, 2024

Yup - grabbing the PR is fine - I'll try to get to it this weekend.

@MasterKale MasterKale unpinned this issue Jan 11, 2024
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 a pull request may close this issue.

2 participants