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

s2clientprotocol stubs #10372

Merged
merged 9 commits into from
Jul 21, 2023
Merged

Conversation

jrtknauer
Copy link
Contributor

@jrtknauer jrtknauer commented Jun 27, 2023

Motivation

Fixes #10371. Should the s2client-proto maintainers respond to and merge my PR as part of the package, then inclusion of the stubs here would no longer be necessary. However, the maintainers do not seem to be particularly active, so I believe it would be better to get the stubs working here first (if it is possible to overlook the discrepencies in the third party stub test).

Stubs were generated using protoc version 3.4.0 and mypy-protobuf.
@github-actions

This comment has been minimized.

Constrain to version 5.* as that is the current of s2clientprotocol
which is being published.

The protobuf version is constrained to <4 for compatability reasons.
While it is unpinned in the requires for s2clientprotocol, all
downstream users actually need to end up pinning to <4 as the protoc
version (3.4.0) generates proto2 messages, which raise runtime errors
when used with protobuf>=4.
@AlexWaygood
Copy link
Member

(Nit: no need to force-push on a typeshed PR. We merge all PRs using GitHub's "squash and merge" button, so we don't really care about a clean commit history, and force pushes tend to make it harder for us to see what changed in between commits :)

@github-actions

This comment has been minimized.

@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, and sorry for the delay! Stubtest struggles a bit with this package, so I've had to add all the generated files to the stubtest allowlist. We already do this quite extensively for other stubs packages that we generate using mypy-protobuf, though, so I'm fine with that. mypy-protobuf generally generates pretty good stubs that users seem to be happy with!

@jrtknauer
Copy link
Contributor Author

No worries. In the interim I have been building the protos and stubs in-project, but there are other s2clientprotocol users who could benefit from having access to stubs.

@github-actions
Copy link
Contributor

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

@AlexWaygood AlexWaygood merged commit 33588c7 into python:main Jul 21, 2023
49 checks passed
AlexWaygood added a commit that referenced this pull request Jul 22, 2023
These stubs were added yesterday in #10372, and I forgot that we had just added this new metadata field
AlexWaygood added a commit that referenced this pull request Jul 22, 2023
These stubs were added yesterday in #10372, and I forgot that we had just added this new metadata field
jrtknauer added a commit to jrtknauer/pycraft2 that referenced this pull request Aug 1, 2023
With the type stubs for s2clientprotocol now available in typeshed [1],
compiling the protocol buffers and the types from the s2client-proto
submodule is no longer necessary.

References:

[1] python/typeshed#10372
jrtknauer added a commit to jrtknauer/pycraft2 that referenced this pull request Aug 1, 2023
With the type stubs for s2clientprotocol now available in typeshed [1],
compiling the protocol buffers and the types from the s2client-proto
submodule is no longer necessary.

References:

[1] python/typeshed#10372
jrtknauer added a commit to jrtknauer/pycraft2 that referenced this pull request Aug 1, 2023
With the type stubs for s2clientprotocol now available in typeshed [1],
compiling the protocol buffers and the types from the s2client-proto
submodule is no longer necessary.

References:

[1] python/typeshed#10372
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.

Create third-party stubs for s2clientprotocol
2 participants