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

Fully drop openssl in favor of cryptography #197

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jvanasco
Copy link
Contributor

@jvanasco jvanasco commented Dec 21, 2024

This is the requested alternative to #196 ; while I was happy to generate this PR, I would rather see a less breaking approach as suggested in the other PR.

The only other thing to note here is changing the test case that proxies attributes from the wrapped object, as Cryptography does not have that particular attribute. In the other PR I calculated it; in this one I just switched to another one. IMHO, I think entirely dropping the attribute proxy is worth considering.

Edit: The approach in #182 is possibly better than this one as it fully drops the ComparableX509 object. If things are going to break, they might as well drop that as it exists to only compare two objects.

@ohemorange
Copy link
Contributor

Thanks for writing up this alternative! I agree that just going ahead and dropping ComparableX509 makes sense; do you think it would be cleaner to do that here or in a separate PR? Not sure which attribute proxying you're referring to the in comment, but seems like it would probably address that as well.

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.

2 participants