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

Interface: Make annotation check optional #5775

Merged
merged 2 commits into from Feb 8, 2021

Conversation

tiran
Copy link
Contributor

@tiran tiran commented Feb 8, 2021

Fixes: #5774
Signed-off-by: Christian Heimes cheimes@redhat.com

Fixes: pyca#5774
Signed-off-by: Christian Heimes <cheimes@redhat.com>
Comment on lines 72 to 83
def strip_annotation(signature):
return inspect.Signature(
[
inspect.Parameter(
param.name,
param.kind,
default=param.default,
annotation=inspect.Parameter.empty,
)
for param in signature.parameters.values()
]
)
Copy link
Contributor

Choose a reason for hiding this comment

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

What about replace methods?
for example,

return signature.replace(                                                                    
    parameters=[                                                                   
        param.replace(annotation=inspect.Parameter.empty)                          
        for param in signature.parameters.values()                                       
    ],                                                                             
    return_annotation=inspect.Signature.empty,                                     
) 

Co-authored-by: Stanislav Levin <slev@altlinux.org>
Signed-off-by: Christian Heimes <cheimes@redhat.com>
@alex
Copy link
Member

alex commented Feb 8, 2021

Interesting, I hadn't realized anyone was using register_interface outside of our tree. WDYT @reaperhulk ?

@reaperhulk
Copy link
Member

If we're breaking people on this we should definitely merge a fix. I'm fine with this approach.

@alex
Copy link
Member

alex commented Feb 8, 2021

Note that this does change the default on whether it verifies sigs, I suppose that's fine?

@tiran tiran marked this pull request as ready for review February 8, 2021 14:05
@reaperhulk reaperhulk merged commit 13e7e56 into pyca:master Feb 8, 2021
@reaperhulk
Copy link
Member

Yeah I'm fine with that to be clear :)

tiran added a commit to tiran/cryptography that referenced this pull request Feb 8, 2021
* Interface: Make annotation check optional

Fixes: pyca#5774
Signed-off-by: Christian Heimes <cheimes@redhat.com>

* Use param.replace()

Co-authored-by: Stanislav Levin <slev@altlinux.org>
Signed-off-by: Christian Heimes <cheimes@redhat.com>

Co-authored-by: Stanislav Levin <slev@altlinux.org>
reaperhulk pushed a commit that referenced this pull request Feb 8, 2021
* Interface: Make annotation check optional

Fixes: #5774
Signed-off-by: Christian Heimes <cheimes@redhat.com>

* Use param.replace()

Co-authored-by: Stanislav Levin <slev@altlinux.org>
Signed-off-by: Christian Heimes <cheimes@redhat.com>

Co-authored-by: Stanislav Levin <slev@altlinux.org>

Co-authored-by: Stanislav Levin <slev@altlinux.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Typing signature breaks register_interface() for custom implementations
4 participants