Skip to content

Fix AWS, GCP, HDFS, Kafka and Ignite disable config options #23568

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

Merged
merged 1 commit into from
Nov 13, 2018

Conversation

MattConley
Copy link
Contributor

The logic implemented in commit 3437098 sets no_<package>_support=true if the no<package> config option is provided, but checks for a false value (in the tensorflow/BUILD file) to disable the package.

This should fully fix Issue #22819

@ymodak ymodak self-assigned this Nov 6, 2018
@ymodak ymodak requested a review from gunan November 6, 2018 23:16
@ymodak ymodak added the awaiting review Pull request awaiting review label Nov 6, 2018
@MattConley
Copy link
Contributor Author

It looks like disabling GCP causes an error when trying to use a package in tensorflow/contrib, as the init file automatically imports cloud regardless of whether the user opted to disable it (https://github.com/tensorflow/tensorflow/blob/master/tensorflow/contrib/__init__.py#L30). Do you have any suggestions as to how we could fix this?

@gunan gunan added awaiting testing (then merge) kokoro:force-run Tests on submitted change and removed awaiting review Pull request awaiting review labels Nov 10, 2018
@kokoro-team kokoro-team removed kokoro:force-run Tests on submitted change labels Nov 10, 2018
@gunan
Copy link
Contributor

gunan commented Nov 10, 2018

We should be able to make the cloud module under contrib non-breaking if gcp is not used.
We may want to merge that one first.

@ymodak ymodak added ready to pull PR ready for merge process and removed awaiting testing (then merge) labels Nov 12, 2018
@tensorflow-copybara tensorflow-copybara merged commit 3914ff7 into tensorflow:master Nov 13, 2018
tensorflow-copybara pushed a commit that referenced this pull request Nov 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes ready to pull PR ready for merge process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants