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

No module named "grpc_interceptor" error #1193

Closed
AmandaHassoun opened this issue Sep 4, 2024 · 12 comments · Fixed by #1197
Closed

No module named "grpc_interceptor" error #1193

AmandaHassoun opened this issue Sep 4, 2024 · 12 comments · Fixed by #1197
Assignees
Labels
api: spanner Issues related to the googleapis/python-spanner API.

Comments

@AmandaHassoun
Copy link

AmandaHassoun commented Sep 4, 2024

Environment details

  • OS type and version:
  • Python version: 3.11
  • google-cloud-spanner version: 3.49.0, so latest release

Steps to reproduce

  1. Attempt to download google-cloud-spanner==3.49.0
  2. Attempt to insert a record into a Spanner instance.

Stack trace

ModuleNotFoundError: No module named 'grpc_interceptor'
Traceback (most recent call last):
  ....
  File "/usr/lib/python3.11/site-packages/google/cloud/spanner.py", line 17, in <module>
    from google.cloud.spanner_v1 import __version__
  File "/usr/lib/python3.11/site-packages/google/cloud/spanner_v1/__init__.py", line 70, in <module>
    from google.cloud.spanner_v1.client import Client
  File "/usr/lib/python3.11/site-packages/google/cloud/spanner_v1/client.py", line 50, in <module>
    from google.cloud.spanner_v1.instance import Instance
  File "/usr/lib/python3.11/site-packages/google/cloud/spanner_v1/instance.py", line 37, in <module>
    from google.cloud.spanner_v1.testing.database_test import TestDatabase
  File "/usr/lib/python3.11/site-packages/google/cloud/spanner_v1/testing/database_test.py", line 25, in <module>
    from google.cloud.spanner_v1.testing.interceptors import (
  File "/usr/lib/python3.11/site-packages/google/cloud/spanner_v1/testing/interceptors.py", line 16, in <module>
    from grpc_interceptor import ClientInterceptor
ModuleNotFoundError: No module named 'grpc_interceptor'

I suspect this is related to this recent commit.

@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/python-spanner API. label Sep 4, 2024
@cchabilall83
Copy link

cchabilall83 commented Sep 4, 2024

I recently encountered the same issue we've using the spanner client with no issue for months and it's failing today.

Python version: 3.10

File "/opt/hostedtoolcache/Python/3.10.11/x64/lib/python3.10/site-packages/google/cloud/spanner.py", line 17, in <module>
    from google.cloud.spanner_v1 import __version__
  File "/opt/hostedtoolcache/Python/3.10.11/x64/lib/python3.10/site-packages/google/cloud/spanner_v1/__init__.py", line 70, in <module>
    from google.cloud.spanner_v1.client import Client
  File "/opt/hostedtoolcache/Python/3.10.11/x64/lib/python3.10/site-packages/google/cloud/spanner_v1/client.py", line 50, in <module>
    from google.cloud.spanner_v1.instance import Instance
  File "/opt/hostedtoolcache/Python/3.10.11/x64/lib/python3.10/site-packages/google/cloud/spanner_v1/instance.py", line 37, in <module>
    from google.cloud.spanner_v1.testing.database_test import TestDatabase
  File "/opt/hostedtoolcache/Python/3.10.11/x64/lib/python3.10/site-packages/google/cloud/spanner_v1/testing/database_test.py", line [25](https://github.com/nicejobinc/ai-ui-agent/actions/runs/10705101662/job/29679667449#step:5:26), in <module>
    from google.cloud.spanner_v1.testing.interceptors import (
  File "/opt/hostedtoolcache/Python/3.10.11/x64/lib/python3.10/site-packages/google/cloud/spanner_v1/testing/interceptors.py", line 16, in <module>
    from grpc_interceptor import ClientInterceptor
ModuleNotFoundError: No module named 'grpc_interceptor'

@potiuk
Copy link
Contributor

potiuk commented Sep 4, 2024

Same for Apache Airflow - https://github.com/apache/airflow/actions/runs/10708266123/job/29690485827#step:7:4358. our tests started to fail after 3.49.0 release today

potiuk added a commit to potiuk/airflow that referenced this issue Sep 4, 2024
Google Cloud Spanner library released today has a missing
grpc_intercept module.

See: googleapis/python-spanner#1193
potiuk added a commit to apache/airflow that referenced this issue Sep 4, 2024
Google Cloud Spanner library released today has a missing
grpc_intercept module.

See: googleapis/python-spanner#1193
@rbarrette
Copy link

Also running into this issue today!

@rbarrette
Copy link

Looks like 3.49.0 moved grpc-interceptor to extras/testing. When I test locally with google-cloud-spanner[testing] in requirements, grpc-interceptor is installed.

@potiuk
Copy link
Contributor

potiuk commented Sep 5, 2024

Looks like 3.49.0 moved grpc-interceptor to extras/testing. When I test locally with google-cloud-spanner[testing] in requirements, grpc-interceptor is installed.

Which is problematic because you do not want all the extra test dependencies when you have "production" dependency specification. For now I just added !=3.49.0 in Airflow and hope it is going to be fixed in 3.49.1 (in which case it will be automatically picked up and tested by Airflow)

@rbarrette
Copy link

rbarrette commented Sep 5, 2024

@potiuk

Here is the specific change. I agree with you when it comes to avoiding test dependency pollution. Looks like that is the only dependency in the testing extra. I opened a PR over at airflow, but might be best to wait and see if the google-cloud-spanner team reverts this change, or commits to this or another path forward.

@potiuk
Copy link
Contributor

potiuk commented Sep 5, 2024

@potiuk

Here is the specific change. I agree with you when it comes to avoiding test dependency pollution. Looks like that is the only dependency in the testing extra. I opened a PR over at airflow, but might be best to wait and see if the google-cloud-spanner team reverts this change, or commits to this or another path forward.

No - no - absolutely. We DON"T want to add test dependency there, so that PR is not going to be accepted. We already have 719 dependencies in airflow and we do not want to add "testing" to be there in the "production" dependencies - and if you do it this way, that is what is going to happen.

The final solution can be one of two things:

  1. If this is a "truly" testing dependency, then it should be added in "devel" section of the provider.yaml (few pages down) - we do NOT want testing dependency to be "regular" dependency for our users.

2)If this is really "necessary" dependency for production - it should not be an extra

Maybe the intention of spanner team is 1) but currently - at least as far as I can tell this is not the case, because spanner library imports the "grpc-interceptor" when just "import google.cloud.spanner_v1.instance happens - this means that (erronously in 3.49.0) "grpc-interceptor" is WRONGLY marked as testing dependncy because it is always required - so it SHOULD be in "Requires" section.

Maybe the future version will change "grpc_interceptor" to be truly optional "test-only" dependency. If that will be the case, we will change our "devel" dependencies to include it - but currently, even if it is marked as "testing", it is "required" - which is the root cause of the problem.

@potiuk
Copy link
Contributor

potiuk commented Sep 5, 2024

Also - to be perfectly blunt - if we workaround it in Airlfow by adding [testing], this will give smaller incentive for the spanner team to fix it. For example if 3.49.1 will also have the same problem and it starts failing our CI, we will change != into <3.49.0 - and until the consistency will be "proper" we will keep it this way. Not mentioning that it puts it on us to remember to remove testing at some point in time because - for example - future versions of spanner library will add more and more dependencies there.

So yeah. Let's wait on how this problem will be solved by the spanner team and for now we just skip 3.49.0 as "buggy" (also my recommendation will be to yank this version when 3.49.1 is released - because as you saw - quite a feew users were affected in similar way.

@rbarrette
Copy link

rbarrette commented Sep 5, 2024

@potiuk I agree with everything you are saying above, looks like the dep change was done due to following issues: 1143 and 1158. I think the short term solution the team should take would be to revert 44434aa and release a 3.49.1 hotfix.

@harshachinta
Copy link
Contributor

harshachinta commented Sep 5, 2024

grpc_interceptor dependency is used only for testing and it made sense to move the dependency from required to extras. However, the reason for failure is importing the testing folders on the main code, which should not have been added in the first place. I will relook on this later.

This PR #1195 should fix this issue. Please let me know your thoughts on this fix. Will this still lead to breakage?

@rbarrette
Copy link

@harshachinta The fix looks acceptable. Curious to hear what @AmandaHassoun and @potiuk think. Would the team be able to yank the bad release as @potiuk mentioned above?

@potiuk, assuming the above approach is taken, would these changes be acceptable in airflow?

@potiuk
Copy link
Contributor

potiuk commented Sep 6, 2024

@potiuk, assuming the above approach is taken, would these changes be acceptable in airflow?

Not exactly those changes but the spirit is the same.

It will have to be done somewhat differntly - not in hatch_build but in provider.yaml - because hatch_build will generate devel dependencies dynamically for google provider based on provider.yaml (and that for example means that when you do:

  • pip install -e ".[google]"

This will install all "google" and all "devel" dependencies of google are also installed (because it's an editable installation)

but on the other hand:

  • pip install "apache-airflow[google]"

Will NOT install those "devel" google dependencies.

So we will have to add "devel-dependencies" similar to "amazon" one here: https://github.com/apache/airflow/blob/main/airflow/providers/amazon/provider.yaml#L139

But we can do it - no problem as soon as new version of spanner library is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment