ray icon indicating copy to clipboard operation
ray copied to clipboard

Uncomment RayDP tests since arrow version limits removed

Open kira-lin opened this issue 2 years ago • 2 comments

Why are these changes needed?

Now that MLDataset has been made optional dependency, and PyArrow version limit has been removed, RayDP tests in ray can be re-enabled now.

Related issue number

Checks

  • [ ] I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • [ ] I've run scripts/format.sh to lint the changes in this PR.
  • [ ] I've included any doc changes needed for https://docs.ray.io/en/master/.
  • [ ] I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • [ ] Unit tests
    • [ ] Release tests
    • [ ] This PR is not tested :(

kira-lin avatar Dec 15 '22 01:12 kira-lin

It seems like java is not installed in the instances where arrow datasets are run. What should I do? @clarkzinzow @jjyao

kira-lin avatar Dec 15 '22 03:12 kira-lin

I found that the ci images is updated here #28641, it seems that java is not installed in ml images. I tried to modify the Dockerfile to install java, but it did not work. What should I do? @krfricke

kira-lin avatar Jan 03 '23 08:01 kira-lin

@krfricke tests has passed, but when I try to add a ci/env/install_java.sh, tests failed due to permission denied. How to solve it?

kira-lin avatar Feb 17 '23 01:02 kira-lin

@krfricke tests has passed, but when I try to add a ci/env/install_java.sh, tests failed due to permission denied. How to solve it?

Did you enable the execution flag with chmod +x install_java.sh on your local machine?

krfricke avatar Feb 17 '23 01:02 krfricke

Did you enable the execution flag with chmod +x install_java.sh on your local machine? @krfricke thanks, the issue has been solved. Now the tests are ok, it's ready to merge

kira-lin avatar Feb 17 '23 05:02 kira-lin

Also, you are building on a pretty old base master branch. Could you please merge the latest upstream master into this PR?

krfricke avatar Feb 17 '23 05:02 krfricke