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

Use pyasn1 in ldap3 #9470

Merged
merged 12 commits into from Jan 11, 2023
Merged

Use pyasn1 in ldap3 #9470

merged 12 commits into from Jan 11, 2023

Conversation

Avasam
Copy link
Sponsor Collaborator

@Avasam Avasam commented Jan 6, 2023

Gonna have to wait for stub-uploader to push types-pyasn1 first.
Dependant on #9471
Closes #9471

Fixes a lot of Any subtyping issues.

Ref: #9491

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@Avasam
Copy link
Sponsor Collaborator Author

Avasam commented Jan 8, 2023

I had the same 2 errors locally. This really looks like a stubtest bug: https://github.com/python/typeshed/actions/runs/3866310116/jobs/6590353764#step:5
(the two types mentionned are not at those lines!)

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks! I've left 3 comments on the first file. It looks like those comments also apply to a lot of the other touched files, so I'll let you respond to those comments before doing a further review :)

stubs/ldap3/ldap3/extend/novell/endTransaction.pyi Outdated Show resolved Hide resolved
stubs/ldap3/ldap3/extend/novell/endTransaction.pyi Outdated Show resolved Hide resolved
stubs/ldap3/ldap3/extend/novell/endTransaction.pyi Outdated Show resolved Hide resolved
@AlexWaygood
Copy link
Member

I had the same 2 errors locally. This really looks like a stubtest bug: https://github.com/python/typeshed/actions/runs/3866310116/jobs/6590353764#step:5 (the two types mentionned are not at those lines!)

Yeah, I can't say I really understand what's going on here. The runtime source code is a bit of a maze, but even if it's doing something extremely dynamic that stubtest can't understand, the fact that stubtest is magicking line numbers out of thin air is a bit strange. Don't know if @hauntsaninja has any theories?

Anyway, for now I guess you should probably just allowlist them :)

@hauntsaninja
Copy link
Collaborator

The bug is just the line number? If so, why does it need an allowlist entry? stubtest's logic for line numbers is a little hacky, looks like it's just the line number from the file that the parent class is in (pyasn1.type.univ)

@github-actions

This comment has been minimized.

@Avasam
Copy link
Sponsor Collaborator Author

Avasam commented Jan 11, 2023

The bug is just the line number? If so, why does it need an allowlist entry? stubtest's logic for line numbers is a little hacky, looks like it's just the line number from the file that the parent class is in (pyasn1.type.univ)

I couldn't find the right type from source. But I got it now

@github-actions

This comment has been minimized.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks! A few more things, but nearly there!

stubs/ldap3/ldap3/extend/novell/getBindDn.pyi Outdated Show resolved Hide resolved
stubs/ldap3/ldap3/core/connection.pyi Outdated Show resolved Hide resolved
stubs/ldap3/ldap3/extend/operation.pyi Outdated Show resolved Hide resolved
stubs/ldap3/ldap3/operation/extended.pyi Outdated Show resolved Hide resolved
stubs/ldap3/ldap3/protocol/rfc4511.pyi Show resolved Hide resolved
stubs/ldap3/ldap3/protocol/rfc4511.pyi Outdated Show resolved Hide resolved
stubs/ldap3/ldap3/protocol/rfc4511.pyi Show resolved Hide resolved
stubs/ldap3/ldap3/utils/asn1.pyi Outdated Show resolved Hide resolved
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks!

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood AlexWaygood merged commit bc08ecf into python:main Jan 11, 2023
@Avasam Avasam deleted the pyasn1-in-ldap3 branch January 11, 2023 23:17
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.

None yet

3 participants