[runtime_env] Remote whl support

[jonathan-anyscale]

Why are these changes needed?

Refactor runtime_env to support remote wheel for py_modules
Unit test: Combined in test_runtime_env_working_dir_remote_uri.py

Related issue number

Closes #35804

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 added any new APIs to the API Reference. For example, if I added a
    method in Tune, I've added it in doc/source/tune/api/ under the
    corresponding .rst file.
• 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 :(

[architkulkarni]

@jonathan-anyscale is this ready for review or still WIP?

[jonathan-anyscale]

@jonathan-anyscale is this ready for review or still WIP?

still WIP, doing some refactor instead of this method and trying to test it right now

[architkulkarni]

Sounds good! You can "mark as draft" in that case https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request#converting-a-pull-request-to-a-draft

[architkulkarni]

Looks good overall, just a few minor comments and questions. @jjyao can you review this as well?

[architkulkarni]

The last round of changes looks good! One question is that now we're allowing a user to set a working_dir to a remote .whl file. Even though it technically makes sense because a whl file happens to be a zip of a directory, I think it might be confusing for users, so it would be better to just disallow it. What do you think? We can always add it later if users ask for it.

Also, please update the documentation in this PR to reflect the change. It's on this page https://docs.ray.io/en/latest/ray-core/handling-dependencies.html

[architkulkarni]

Looks like one or more tests may need to be updated, e.g.: https://buildkite.com/ray-project/oss-ci-build-pr/builds/36008#018a8c11-1593-4e10-9d3d-11694b724e24/3606-4413

[jonathan-anyscale]

The last round of changes looks good! One question is that now we're allowing a user to set a working_dir to a remote .whl file. Even though it technically makes sense because a whl file happens to be a zip of a directory, I think it might be confusing for users, so it would be better to just disallow it. What do you think? We can always add it later if users ask for it.

Also, please update the documentation in this PR to reflect the change. It's on this page https://docs.ray.io/en/latest/ray-core/handling-dependencies.html

Okk make sense, I didn't check before that local whl is not supported for working_dir as well. So I agree with disallowing it as user can just fill py_modules field for .whl file

[architkulkarni]

Looks good to me! If there are failing tests that look unrelated, you can check against https://flaky-tests.ray.io/ to see if they're flaky on master, list out the ones that failed and mark the PR with the tests-ok.

[angelinalg]

Just had a few minor nits.

[jjyao]

Will this cause conflict since we only use the filename of whl instead of the entire uri? Like if I have both s3:https://A/package.whl and s3:https://B/package.whl.

[jonathan-anyscale]

Should be fine since whl filename is uniquely generated when we create it. The only conflict is when the user want to use package with same name which is unlikely.
Also, this filename will only used to save locally, which the file will be deleted after installation so shouldn't be any conflict.

[jjyao]

why do we move to here?

[jonathan-anyscale]

Initially I thought working_dir will support remote wheel as well but it isn't by our definition of working_dir.
But this function is generally used to install wheel package, so might as well move it outside from py_modules for general usage later.