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

Fixing typing for namespaced aio import #52

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

macro1
Copy link

@macro1 macro1 commented Oct 18, 2023

Description of change

Minimum Reproducible Example

Pull requests of any significance will not be accepted without minimum reproducible examples.
If your contribution is small enough (a few lines tops), a typing test might be enough, but an MRE might still
be requested. If in doubt, presume an MRE will be required.

Pull requests where an MRE is requested but not supplied will be closed.

"Reproducible" means you have given me enough materials to exercise runnable code that
demonstrates your change, on my local machine, with a minimum of fuss.

Gists, links to other repositories, or typing tests are not acceptable as an MRE.

If an MRE is needed (it probably is), the following must be provided, at a minimum, in
separate <details> blocks, using as few files as possible (templates are provided below):

  • One or more python files containing reproducing code.
  • Full set of shell commands (POSIX shell or bash only) required to create a venv, install
    dependencies, and generate proto.

Please see README.md for the rationale for why this is needed.

main.py
# Full python code to reproduce
if __name__ == "__main__":
    import grpc
    grpc.aio.Channel
run.sh
#!/usr/bin/env bash
set -o errexit -o nounset -o pipefail
python -m venv venv
source ./venv/bin/activate
pip install grpcio
python main.py

Checklist:

  • I have verified my MRE is sufficient to demonstrate the issue and solution by attempting to execute it myself
    • OR I believe my contribution is minor enough that a typing test is sufficient.
  • I have added tests to typesafety/test_grpc.yml for all APIs added or changed by this PR
  • I have removed any code generation notices from anything seeded using mypy-protobuf.

@shabbyrobe
Copy link
Owner

Thanks for the report. After giving up on the gRPC documentation and just searching on Github a bit, I see a lot of examples that just use import grpc.aio to accomplish this. Please provide some citations or better MREs if you still think this should be merged. Closing for now.

@shabbyrobe shabbyrobe closed this Dec 27, 2023
@kandji-micah
Copy link

kandji-micah commented Dec 28, 2023

Hi @shabbyrobe. I don't know what you mean by better MREs. The code will execute correctly in the example I provided, but type-checking will fail. I view that as a gap in grpc-stubs.

Although, I can appreciate not caring if the convention is to use the library in a way that avoids the issue. Honestly, I only ran into it because I was working on cross-compatible code with sync and async.

The code in the grpc package that causes the gap between grpc and grpc-stubs is here:
https://github.com/grpc/grpc/blob/b25a382b602aabbd99439261f88cf05728988207/src/python/grpcio/grpc/__init__.py#L2307

Edit: Apologies. I just realized I'm on my work account at the moment haha.

@shabbyrobe
Copy link
Owner

The citation is sufficient.

@shabbyrobe shabbyrobe reopened this Dec 28, 2023
@shabbyrobe shabbyrobe merged commit 32ba796 into shabbyrobe:master Dec 28, 2023
8 checks passed
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.

3 participants