-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
[ci] migrate remaining non-gpu ml tests to civ2 #40527
Conversation
a75ab7f
to
208522a
Compare
./ci/env/install-dependencies.sh | ||
INSTALL_HDFS=1 ./ci/env/install-dependencies.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the only new line here
ci/ray_ci/tests.env.Dockerfile
Outdated
@@ -8,6 +8,8 @@ ARG BUILD_TYPE | |||
ENV CC=clang | |||
ENV CXX=clang++-12 | |||
|
|||
SHELL ["/bin/bash", "-ice"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is required to use oss-ci-base_build for testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why? what is the error?
rayci already sets shell to -elic
for the docker plugin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah -elic is only provided during docker run; this is docker build; the oss_ci_base_build is already have pip installed (through ci/env/install-dependencies.sh i assume), but without this the docker build process will say pip not found (assume pip is loaded through .bashrc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the build, it has !/bin/bash
shebang. I think the only thing you want is -i
, which adds anaconda's pip
into PATH
, you do not want to enable -c
and -e
by default though. that would be quite surprising result for users.
it might be easier and cleaner to add $HOME/anaconda3/bin/
into PATH (export PATH="$HOME/anaconda3/bin:$PATH"
at the beginning of the script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping only -i
sounds good, I want to load the .bashrc instead of only getting pip, there are quite a few places setting things up in .bashrc (https://github.com/ray-project/ray/blob/master/.buildkite/pipeline.ml.yml#L367).
A few things at put in root/.bashrc, others are in ~/.bashrc, so -i
will be a safer option to set all things up.
208522a
to
1dd7f69
Compare
- bazel run //ci/ray_ci:test_in_docker -- //python/ray/tune/... ml | ||
--only-tags soft_imports | ||
--build-name oss-ci-base_build | ||
depends_on: oss-ci-base_build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this image, the default shell is sh not bash
bc561c7
to
cb59369
Compare
@aslonnie I still need to use at least |
25559c9
to
fa7d9de
Compare
fa7d9de
to
13d1830
Compare
Signed-off-by: can <[email protected]>
13d1830
to
3feb905
Compare
Test: