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

Travis CI Enhancement #94

Merged
merged 9 commits into from
Feb 18, 2019
Merged

Travis CI Enhancement #94

merged 9 commits into from
Feb 18, 2019

Conversation

yongtang
Copy link
Member

@yongtang yongtang commented Feb 15, 2019

This is a Travis CI rework. The idea is to split C++ build from Python and R build. Also, bazel will only be used for C++ build, and python testing has been switched to pytest.

[Update ] This PR is now ready. The Travis CI now spend around 15 min each and total (3 jobs) is about 1 hour. This is about 25% of our previous Travis CI CPU usage.

Major changes:

  1. All kernel_tests files are moved to tests, so that it is possible to run python -m pytest tests.
  2. Bazel is only used for C++ build
  3. Removed shell script to generate the whl files.
    Instead, all are packed into setup.py (with setuptoools.sandbox.run_setup)
  4. Travis CI will tests building on Ubuntu 16.04 and 18.04, to make sure there is an easy local development environment.
  5. Travis CI will build the binary release in Ubuntu 14.04, to match tensorflow.
  6. Travis CI will build the binary release in Ubuntu 14.04, and tests both R and Python in Ubuntu 16.04 and 18.04.

Note: The above 6) means:

  1. We support Ubuntu 16.04 and 18.04 with CI testing for every commit.
  2. The tensorflow-io should run on any Linux platform that tensorflow runs, though they are not fully tested by Travis CI. (like CentOS or Ubuntu 14.40).
  3. Supported Python version are: 2.7, 3.4, 3.5, 3.6

We should provide Python 3.7 support to align TF 2.0, not there yet.

Some major changes related to development:

  1. To build tensorflow-io locally (Ubuntu 14.04/16.04/18.04):
    $ bazel build --spawn_strategy standalone --verbose_failures //tensorflow_io/...
    $ python3 setup.py --binary bazel-bin bdist_wheel
    $ auditwheel repair dist/*.whl
    
    Note our release is based on Ubuntu 14.04 to make sure C/C++ runtime version is lower, and runs in newer versions of Linux. But you could build tensorflow-io with any versions of Linux for your own use.
  2. To test tensorflow-io:
    Pretty much switched from bazel to pytest, which is common in python projects. It is possible to test with out packaging the whl file and install:
    $ bazel build --spawn_strategy standalone --verbose_failures //tensorflow_io/...
    $ python3 -m pytest -s --binary tests
    
  3. To release tensorflow-io:
    $ docker run -i -t --rm -v $PWD:/v -w /v --net=host ubuntu:14.04 /v/.travis/python.release.sh
    
    This will build Python 2.7, 3.4, 3.5, 3.6.

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

@yongtang yongtang force-pushed the travis branch 13 times, most recently from 129ace2 to 5c4d997 Compare February 15, 2019 21:45
This is a Travis CI Rework. The idea is to split C++ build
with Python and R build.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang force-pushed the travis branch 7 times, most recently from a38656a to 80d1191 Compare February 16, 2019 22:22
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang force-pushed the travis branch 4 times, most recently from 68aa6b4 to 5fbcbd0 Compare February 17, 2019 04:39
So that we could use `(cd tests && python -m pytest -s .)`

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
to run `python -m pytest -s tests` without install.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang force-pushed the travis branch 3 times, most recently from decc60a to ff81ef8 Compare February 17, 2019 07:00
Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang changed the title [WIP] Travis CI Enhancement Travis CI Enhancement Feb 17, 2019
@yongtang
Copy link
Member Author

@dmitrievanthony @terrytangyuan @BryanCutler This PR should be ready now.

The testing is closer to pythonic with pytest I guess.

Also dropped shell script for whl generation. All are done through python setup.py bdist_wheel --binary bazel-bin.

@yongtang
Copy link
Member Author

Once this PR is merged, I will update the 1.13 support PR. 1.13 support is almost ready as well. We will need 1.13 support soon to match the TF release, which likely will happen in a week or two.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
@yongtang yongtang mentioned this pull request Feb 17, 2019
echo "options(repos = c(CRAN='http://cran.rstudio.com'))" >> ~/.Rprofile
R -e 'install.packages(c("tensorflow", "tfdatasets"), quiet = TRUE)'
R -e 'install.packages(c("testthat", "devtools"), quiet = TRUE)'
R -e 'install.packages(c("forge"), quiet = TRUE)'
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason for splitting them into three different commands?

Copy link
Member Author

Choose a reason for hiding this comment

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

@terrytangyuan Travis CI has a limit of several minutes to see any output, otherwise it will assume the job hangs and cancel. Travis CI also has a limit of max length of logs. Previously we install all packages in one command without quiet. That caused the max length of logs reached. So changed to one command installing every package with quiet = TRUE. But that caused several minutes without any output to stderr or stdout. So I split them into several, just to show something in the output (by command itself echoed in shell, not the install.packages).

R -e 'install.packages(c("testthat", "devtools"), quiet = TRUE)'
R -e 'install.packages(c("forge"), quiet = TRUE)'

V=$(python3 -c 'import sys; print(str(sys.version_info[0])+str(sys.version_info[1]))')
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a more meaningful variable name here?

Copy link
Member Author

Choose a reason for hiding this comment

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

@terrytangyuan Update to use CPYTHON_VERSION to make it easy to read.


# NOTE: __datapath__ is a hidden value used by test,
# in case .so files are on a different path.
__datapath__ = None
Copy link
Member

Choose a reason for hiding this comment

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

Why would .so files be on different paths?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is to allow us to test two scenarios:

  1. Install package from whl files. In this situation, .so file will be part of the path so __datapath__ will always be None, no change needs.
  2. Not install the package (in development). In this situation, as we split the C++ build to use bazel and pytest for python, the .so files are generated in bazel-bin and the python code are in the original source tree. However, bazel does not allow us to write .so file into the original source tree. So __datapath__ is added to capture bazel-bin.

This is mainly for easy local development:

$ bazel build .....
$ python -m pytest tests --binary bazel-bin -s

The above will be all we need for local development and debugging. No installation required.

to address review feedback

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

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

Sounds good. Thanks for the clarification!

@terrytangyuan terrytangyuan merged commit a17e000 into tensorflow:master Feb 18, 2019
@yongtang yongtang deleted the travis branch February 18, 2019 17:52
@yongtang yongtang mentioned this pull request Feb 18, 2019
@BryanCutler
Copy link
Member

This looks great, thanks for the effort @yongtang ! Trying it out now

@yongtang
Copy link
Member Author

@BryanCutler Here is the complete script I used to do a local development:

# docker run -i -t --rm -v $PWD:/v -w /v --net=host ubuntu:18.04
$ apt-get -y -qq update && apt-get -y -qq install python-pip curl unzip patch                         
$ curl -sOL https://github.com/bazelbuild/bazel/releases/download/0.20.0/bazel-0.20.0-installer-linux-x86_64.sh
$ bash -x bazel-0.20.0-installer-linux-x86_64.sh
$ ./configure.sh
$ bazel build -s --verbose_failures --spawn_strategy standalone //tensorflow_io/...
$ python -m pip install pytest
$ python -m pytest tests/test_lmdb.py --binary bazel-bin -s

Note with the new changes it is not necessary to build with custom-op. I am using Ubuntu 18.04.

Let me know if you encounter any issues.

@BryanCutler
Copy link
Member

Yeah, works great! Would you mind if I put the above commands in a script and update the Developing section of the README.md?

@yongtang
Copy link
Member Author

@BryanCutler Sure, that would be great!

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