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

Add google-auth-httplib2 as dependency #2384

Merged
merged 1 commit into from
Apr 5, 2018
Merged

Add google-auth-httplib2 as dependency #2384

merged 1 commit into from
Apr 5, 2018

Conversation

honnix
Copy link
Member

@honnix honnix commented Apr 4, 2018

Description

Adding a new dependency.

Motivation and Context

As discussed here #2361 (comment)

This is used by https://github.com/google/google-api-python-client/blob/v1.6.6/googleapiclient/discovery.py#L371

Another option is to fix luigi.contrib.bigquery somehow.

Have you tested this? If so, how?

Manually installing this package worked for me.

@honnix honnix merged commit 670e00e into master Apr 5, 2018
@honnix honnix deleted the honnix-patch-1 branch April 5, 2018 12:24
@ulzha
Copy link
Contributor

ulzha commented Apr 12, 2018

tox.ini is just adding it as a dependency for tests; didn't you mean to add it as a dependency for the real thing?

It's a bit unfortunate that contrib modules aren't separate packages with their own setup.pys; that would be the place to declare dependencies...

@honnix
Copy link
Member Author

honnix commented Apr 12, 2018

I did mean to put it as real thing. Sorry for the mistake. Another PR coming.

@honnix
Copy link
Member Author

honnix commented Apr 12, 2018

Now the problem is, I don't know where to put it. setup.py doesn't seem to be a good place. @ulzha as you mentioned, how do we handle contrib deps?

@Tarrasch
Copy link
Contributor

As you maybe have noticed by now, we only list the dependencies used in "core" luigi in steup.py here. It's not the best solution, but it have worked so far. I would be really glad if we did manage to create packages for the various contrib packages, at least the larger ones. I tried once but never really got started on it on #2170.

@honnix
Copy link
Member Author

honnix commented Apr 13, 2018

That looks like a big change in code structure. Not so sure we want to spend that much effort atm.

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

4 participants