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.

Sign up for GitHub

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

Jump to bottom

Allow configuring gcs file system in runtime. #1443

Merged
merged 6 commits into from
Jun 30, 2021
Merged

Allow configuring gcs file system in runtime. #1443

merged 6 commits into from
Jun 30, 2021

Conversation

yongtang
Copy link
Member

@yongtang yongtang commented Jun 1, 2021

This is a working in progress PR to have set_configuration support for configuring gcs in runtime.

This PR depends on tensorflow/tensorflow#49951

FYI @vnvo2409

Signed-off-by: Yong Tang yong.tang.github@outlook.com

@yongtang
Work in Progress
c2a3637 
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang
Fix
e7cd4c0 
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang
Fix lint
bcd909c 
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang
Update WORKSPACE
4ba41ef 
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang
Remove TF_USE_MODULAR_FILESYSTEM for s3 and hdfs as they are not part…
c40207c 
… of the tensorflow package anymore

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang
Remove TF_USE_MODULAR_FILESYSTEM for tensorflow_io_gcs_filesystem as …
f172902 
…well,

as tensorflow_io_gcs_filesystem will only be imported explicitly
so no need to use TF_USE_MODULAR_FILESYSTEM to switch on and off.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang marked this pull request as ready for review June 30, 2021 02:11
@yongtang
Copy link
Member Author

This PR is ready for review now. The tensorflow dependency has also been bumped to 2.6.0rc0 to match the upcoming 2.6.0 release.

@yongtang
Copy link
Member Author

/cc @kvignesh1420 @vnvo2409 FYI

@yongtang yongtang changed the title [Work in Progress] Allow configuring gcs file system in runtime. Allow configuring gcs file system in runtime. Jun 30, 2021
@yongtang
Copy link
Member Author

With tensorflow 2.6.0 the s3 and hdfs only exists on tensorflow-io so we do want to make sure everything works fine.

kvignesh1420
kvignesh1420 approved these changes Jun 30, 2021
View reviewed changes
Copy link
Member

@kvignesh1420 kvignesh1420 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yongtang this looks good. Thanks!!

@kvignesh1420
Copy link
Member

With tensorflow 2.6.0 the s3 and hdfs only exists on tensorflow-io so we do want to make sure everything works fine.

we can update our tests to verify the "configuration on fly" op for s3 and hdfs as well. WDYT?

@yongtang yongtang merged commit 16d6f43 into tensorflow:master Jun 30, 2021
@yongtang yongtang deleted the filesystem branch June 30, 2021 15:58
@yongtang
Copy link
Member Author

@kvignesh1420 Yes. In fact starting tensorflow 2.6, user will have to install tensorflow-io to get S3 and HDFS support. And they don't need TF_USE_MODULAR_FILESYSTEM=1 to switch on/off.

TF_USE_MODULAR_FILESYSTEM=1 is only needed for gcs switch (we plan on also remove once we can get GCS/TPU in place within modular file system).

We will need to make sure we can release tensorflow-io 0.20.0 (next version) the same time tensorflow 2.6 is released.

@kvignesh1420 kvignesh1420 mentioned this pull request Jun 30, 2021
Merged
@jsimsa
Copy link
Contributor

jsimsa commented Sep 22, 2021

@yongtang Could you please contribute the Python API and C++ operation for setting file system options introduced by this PR to the core TF repository? There is a need for that API internally and it would be more convenient for that API to exist in core TF instead of tensorflow_io. Thank you.

@yongtang
Copy link
Member Author

@jsimsa Sure I can take a look.

@jsimsa
Copy link
Contributor

jsimsa commented Sep 28, 2021

Hi @yongtang, do you have an ETA for this? This is currently blocking an internal user.

@yongtang
Copy link
Member Author

@jsimsa I am on the road this week but will be back during the weekend. I will work on it starting next week.

@jsimsa
Copy link
Contributor

jsimsa commented Sep 30, 2021

Thank you for the update Yong.

yongtang added a commit to yongtang/tensorflow that referenced this pull request Oct 10, 2021
@yongtang
Add support of filesystem_set_configuration to tensorflow core
5fe98ec 
This is a follow up to PR in tensorflow/io#1443
to add set_configuration support to tensorflow core repo.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
yongtang added a commit to yongtang/tensorflow that referenced this pull request Oct 10, 2021
@yongtang
Add support of filesystem_set_configuration to tensorflow core
13de942 
This is a follow up to PR in tensorflow/io#1443
to add set_configuration support to tensorflow core repo.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang mentioned this pull request Oct 10, 2021
Merged
yongtang added a commit to yongtang/tensorflow that referenced this pull request Oct 10, 2021
@yongtang
Add support of filesystem_set_configuration to tensorflow core
1d56507 
This is a follow up to PR in tensorflow/io#1443
to add set_configuration support to tensorflow core repo.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
yongtang added a commit to yongtang/tensorflow that referenced this pull request Oct 11, 2021
@yongtang
Add support of filesystem_set_configuration to tensorflow core
a11f081 
This is a follow up to PR in tensorflow/io#1443
to add set_configuration support to tensorflow core repo.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this pull request Oct 28, 2021
@jsimsa @tensorflower-gardener
Rollback of PR #52316
f75c3ec 
PR #52316: Add support of filesystem_set_configuration to tensorflow core

Imported from GitHub PR #52316

This is a follow up to PR in tensorflow/io#1443 to add set_configuration support to tensorflow core repo.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Copybara import of the project:

--
a11f081 by Yong Tang <yong.tang.github@outlook.com>:

Add support of filesystem_set_configuration to tensorflow core

This is a follow up to PR in tensorflow/io#1443
to add set_configuration support to tensorflow core repo.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

--
eb8559d by Yong Tang <yong.tang.github@outlook.com>:

Expose tf.experimental.filesystem_set_configuration API

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

--
16ad176 by Yong Tang <yong.tang.github@outlook.com>:

Add placeholder SetOption for gcs file system

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
PiperOrigin-RevId: 406210322
Change-Id: Ia1fd891a522c6f6a81bfab5c86c00e9940b1084a
dopiera pushed a commit to Unoperate/tensorflow_io that referenced this pull request Dec 3, 2021
@yongtang
Allow configuring gcs file system in runtime. (tensorflow#1443)
472d5e2 
* Work in Progress

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

* Fix

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

* Fix lint

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

* Update WORKSPACE

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

* Remove TF_USE_MODULAR_FILESYSTEM for s3 and hdfs as they are not part of the tensorflow package anymore

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

* Remove TF_USE_MODULAR_FILESYSTEM for tensorflow_io_gcs_filesystem as well,

as tensorflow_io_gcs_filesystem will only be imported explicitly
so no need to use TF_USE_MODULAR_FILESYSTEM to switch on and off.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kvignesh1420 kvignesh1420 kvignesh1420 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@yongtang @kvignesh1420 @jsimsa