Skip to content

Conversation

@vlasenkoalexey
Copy link
Contributor

vlasenkoalexey and others added 2 commits November 12, 2019 11:26
…r null values (tensorflow#626)

* making sure that bigquery reader is returning meaningfull defaults for null values

* linter fixes
)

We use sharding to provide a way of shuffling the read order. With liquid sharding, a single stream can read all data which wouldn't be shuffled.
@vlasenkoalexey vlasenkoalexey changed the title making sure that bigquery reader is returning meaningfull defaults for null values (#626) Patching 0.8.0 release Nov 12, 2019
@vlasenkoalexey vlasenkoalexey requested a review from a team November 12, 2019 20:46
@yongtang
Copy link
Member

@vlasenkoalexey overall looks good to me.

Since tensorflow-io 0.8.0/0.9.0, our way of release is to let Travis CI automatically build all platforms and push "release candidates" to a dropbox location:

https://www.dropbox.com/sh/dg0npidir5v1xki/AACor-91kbJh1ScqAdYpxdEca?dl=0

The detail of the release processing is in https://github.com/tensorflow/community/blob/master/sigs/io/RELEASE.md

The script to push to dropbox location is in:

io/.travis.yml

Line 67 in 98f52b6

after_success: bash -x -e .travis/after-success.sh

then =>

https://github.com/tensorflow/io/blob/98f52b669496602757ee801afa6b91fab041482b/.travis/after-success.sh

However, 0.8.0 and 0.9.0 was built from master branch so in after-success.sh:

if [[ ( ${TRAVIS_BRANCH} == "master" ) && ( ${TRAVIS_EVENT_TYPE} != "pull_request" ) ]]; then

as you could see only master branch has been covered.

Can you also change the above line to cover the branch R0.81 and R0.91? Then I think the build will work correctly and all binaries could be pulled from the dropbox location.

@vlasenkoalexey
Copy link
Contributor Author

Thanks @yongtang, great idea, hope I will be able to use binaries from dropbox and won't have to build macos whls manually. I didn't know about https://github.com/tensorflow/community/blob/master/sigs/io/RELEASE.md
BTW, looks like latest R version which was released is 0.4

@yongtang
Copy link
Member

Thanks @vlasenkoalexey. R may need big changes for 2.0 (the binding was build around 1.x before) so it is stalled. We will pick up and update the R once the APIs in tensorflow-io are relatively stable. /cc @terrytangyuan.

@yongtang
Copy link
Member

@vlasenkoalexey can you also bump the version in:
https://github.com/tensorflow/io/blob/ad33b20cb703b1d8a2eebdb30c41101208bb2940/tensorflow_io/core/python/ops/io_info.py

I think both 0.8.1 and 0.9.1 branch may need to be updated.

Copy link
Member

@yongtang yongtang left a comment

Choose a reason for hiding this comment

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

LGTM

@yongtang
Copy link
Member

@vlasenkoalexey The PR looks good, though the Travis CI is failing due to the cache issue. We are using Travis CI to build wheel files on all platforms (Python 2.7/3.5/3.6 for Linux and macOS) so this is an issue for us.

To summarize:

  1. Our code base needs more than 50 mins to build on Travis CI. Travis CI machine is not powerful enough, and our code base of C++ is big.
  2. In the past we largely addresses the issue by utilizing Bazel remote cache to avoid hitting the 50 min limit.
  3. The bazel remote cache is done through BAZEL_CACHE=.cache/bazel which triggers a local file bazel cache on Travis CI. And we save the local bazel file cache to Travis CI's cache.
  4. However, if build job is not done in 50 minutes, Travis CI will not copy the cache (consists of bazel cache files) to their storage. So this is a chicken-and-egg issue.
  5. Ideally we are looking for other sources of cache. SIG Build team offered some help. One idea is that we will get a gcs address and use it as a bazel cache (vs. local bazel cache then save it with Travis CI). However, this is not there yet. (During the Contributor Summit we discussed this issue again, I think we may get there at some point.)

The master branch is OK because the cache in Travis CI is generated gradually. But since R0.81 and R0.91 was based on old files, cache on Travis CI may have be dropped.

At the moment, the way I can think of to get around this issue, (have done that before), is to:

  1. Split bazel build targets into several batches.
  2. Enable one batch of the bazel build target, so that the build on Travis CI complete in less than 50 min no matter what. That will build up some cache on Travis CI.
  3. Enable another batch, so more cache will be built up.
  4. Enable all batches and build will finish in less than 50 min.

The above is very clumpy though not sure we have other ways to get around it. Hopefully SIG Build team can help us with the gcs directory (as a bazel cache) soon.

Or, we could just try to build everything locally. Linux is not an issue as it uses docker container so we could easily build all versions of wheel files.

On mac we may need to build both python 2 and 3. It could be a little annoying though.

@vlasenkoalexey vlasenkoalexey merged commit 0947aca into tensorflow:R0.81 Nov 14, 2019
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.

3 participants