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

BigQuery: doesn't declare dependency on concurrent.futures #4337

Closed
drigz opened this issue Nov 3, 2017 · 4 comments
Closed

BigQuery: doesn't declare dependency on concurrent.futures #4337

drigz opened this issue Nov 3, 2017 · 4 comments
Assignees
Labels
api: bigquery Issues related to the BigQuery API. type: question Request for information or clarification. Not an issue.

Comments

@drigz
Copy link

drigz commented Nov 3, 2017

bigquery/client.py imports concurrent.futures but it doesn't declare the dependency in setup.py. It should have a declaration like the one from google-cloud-core==0.27.1 (removed in 0.28.0).

Currently this is doesn't break anything because api-core depends on futures, but if that dependency disappears it will break.

This may be true for other clients, I haven't checked. I ran into it when looking at bazelbuild/rules_python#14.

@dhermes dhermes added the api: bigquery Issues related to the BigQuery API. label Nov 3, 2017
@dhermes
Copy link
Contributor

dhermes commented Nov 3, 2017

@jonparrott (our resident packaging expert) WDYT here?

I am inclined to say this is "working as intended".

@dhermes dhermes added the type: question Request for information or clarification. Not an issue. label Nov 3, 2017
@theacodes
Copy link
Contributor

Yes, I consider this working as intended. BigQuery does directly import concurrent.futures, so an argument could be made for having a direct dependency.

@lukesneeringer
Copy link
Contributor

Hold on: concurrent.futures is not in the standard library in Python 2.7.

That means that either the BigQuery library only supports 3.4+, or we do need this, yes?

$ python2.7
Python 2.7.10 (default, Feb  7 2017, 00:08:15)
[GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.34)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from concurrent.futures import Future
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: No module named concurrent.futures
>>> ^D

@lukesneeringer lukesneeringer reopened this Nov 3, 2017
@lukesneeringer
Copy link
Contributor

Oh wait, I spoke too soon. Transitive dependency.

I do not love this generally, but considering we control google.api_core and our tests will fail if we ever make a change, yes this is fine.

If we did not control the dependency, it would not be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the BigQuery API. type: question Request for information or clarification. Not an issue.
Projects
None yet
Development

No branches or pull requests

4 participants