[Feat][Core] Implement single file module for runtime_env #47807

MortalHappiness · 2024-09-24T10:37:37Z

Why are these changes needed?

See the description in the corresponding issue for details.

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 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 :(

rynewang

Generally LGTM. Left with some wording issues. Also can you add a real Ray running test in python/ray/tests/test_runtime_env_working_dir.py ?

python/ray/_private/runtime_env/packaging.py

python/ray/_private/runtime_env/py_modules.py

MortalHappiness · 2024-09-25T08:51:22Z

@rynewang

The following screencast shows that the zip file actually only contains single file even if its parent folder contains multiple files.

2024-09-25.16-43-02.mp4

Explanation:

ls _test shows that there are multiple files under _test folder
Also you can see the content of test.py on the right panel, which imports file_module and then inject it as runtime_env.
In IDE, I set a breakpoint after create_package
When the execution reaches the breakpoint, I copied the value of package_file
Also note that the value of the variable directory is a .py file. This variable naming is confusing, so I've renamed it to module_path
I ran a cp command to copy it to my ~/Downloads folder.
I opend that zip file with file manager and you can see that there is only file_module.py in it.

I've renamed some variables and add more comments in 2957106 to make it clear.

MortalHappiness · 2024-09-25T09:46:55Z

Added tests for single file module in python/ray/tests/test_runtime_env_working_dir.py in commit a0d5bc2

rynewang

LGTM! Other than 1 nit on naming we can merge this.

python/ray/_private/runtime_env/packaging.py

Closes: ray-project#23151 Signed-off-by: Chi-Sheng Liu <[email protected]>

Signed-off-by: Chi-Sheng Liu <[email protected]>

Closes: ray-project#23151 Signed-off-by: Chi-Sheng Liu <[email protected]>

…king_dir.py Closes: ray-project#23151 Signed-off-by: Chi-Sheng Liu <[email protected]>

Closes: ray-project#23151 Signed-off-by: Chi-Sheng Liu <[email protected]>

rynewang · 2024-09-26T22:22:01Z

@angelinalg Hi this PR updates the py_modules runtime env. Previously it only support a py module as a directory, now it supports the other type of module: a single file. Please check the docs, thanks!

angelinalg

Some style nits and some clarification for confusing sentence. Please correct if I introduced inaccuracies.

angelinalg · 2024-10-11T20:09:43Z

python/ray/_private/runtime_env/packaging.py

+) -> bytes:
+    """Helper function to create hash of a single file or directory.
+
+    This function will hash the path of the file or directory,


Suggested change

This function will hash the path of the file or directory,

This function hashes the path of the file or directory,

angelinalg · 2024-10-11T20:10:05Z

python/ray/_private/runtime_env/packaging.py

+    """Helper function to create hash of a single file or directory.
+
+    This function will hash the path of the file or directory,
+    and if it's a file, it'll hash its content too.


Suggested change

and if it's a file, it'll hash its content too.

and if it's a file, then it hashes its content too.

angelinalg · 2024-10-11T20:10:23Z

python/ray/_private/runtime_env/packaging.py

+        except Exception as e:
+            logger.debug(
+                f"Skipping contents of file {filepath} when calculating package hash "
+                f"because the file could not be opened: {e}"


Suggested change

f"because the file could not be opened: {e}"

f"because the file couldn't be opened: {e}"

angelinalg · 2024-10-11T20:10:35Z

python/ray/_private/runtime_env/packaging.py

+) -> bytes:
+    """Helper function to create hash of a single file.
+
+    It'll hash the path of the file and its content to create a hash value.


Suggested change

It'll hash the path of the file and its content to create a hash value.

It hashes the path of the file and its content to create a hash value.

angelinalg · 2024-10-11T20:11:04Z

python/ray/_private/runtime_env/packaging.py

+def get_uri_for_file(file: str) -> str:
+    """Get a content-addressable URI from a file's content.
+
+    This function will generate the name of the package by the file.


Suggested change

This function will generate the name of the package by the file.

This function generates the name of the package by the file.

angelinalg · 2024-10-11T20:12:52Z

python/ray/_private/runtime_env/packaging.py

+    """Get a content-addressable URI from a file's content.
+
+    This function will generate the name of the package by the file.
+    The final package name is: _ray_pkg_<HASH_VAL>.zip of this package.


Suggested change

The final package name is: _ray_pkg_<HASH_VAL>.zip of this package.

The final package name is _ray_pkg_<HASH_VAL>.zip, where <HASH_VAL> is a hash of this package.

I couldn't understand what you meant to say in this sentence. This is a guess at a rewrite. Please correct if needed.

angelinalg · 2024-10-11T20:13:31Z

python/ray/_private/runtime_env/packaging.py

+
+    This function will generate the name of the package by the file.
+    The final package name is: _ray_pkg_<HASH_VAL>.zip of this package.
+    e.g., _ray_pkg_029f88d5ecc55e1e4d64fc6e388fd103.zip


Suggested change

e.g., _ray_pkg_029f88d5ecc55e1e4d64fc6e388fd103.zip

For example: _ray_pkg_029f88d5ecc55e1e4d64fc6e388fd103.zip

rynewang · 2024-10-11T20:25:41Z

Oops! Auto-merged without applying the doc reviews. @MortalHappiness would you mind updating the docs in another PR?

…t#47807) Supports single file modules in `py_module` runtime_env. Signed-off-by: Chi-Sheng Liu <[email protected]> Signed-off-by: ujjawal-khare <[email protected]>

MortalHappiness force-pushed the bugfix/#23151-python-modules branch 2 times, most recently from e6afa5d to 175cdf5 Compare September 24, 2024 11:54

MortalHappiness marked this pull request as ready for review September 24, 2024 11:54

MortalHappiness requested a review from a team as a code owner September 24, 2024 11:55

MortalHappiness requested a review from rynewang September 24, 2024 11:55

anyscalesam added triage Needs triage (eg: priority, bug/not-bug, and owning component) core Issues that should be addressed in Ray Core labels Sep 24, 2024

kevin85421 assigned rynewang Sep 24, 2024

rynewang reviewed Sep 24, 2024

View reviewed changes

MortalHappiness force-pushed the bugfix/#23151-python-modules branch from 9acb04b to 2957106 Compare September 25, 2024 08:56

rynewang approved these changes Sep 25, 2024

View reviewed changes

python/ray/_private/runtime_env/packaging.py Outdated Show resolved Hide resolved

MortalHappiness added 7 commits September 26, 2024 06:41

[Docs][Core] Update docs for single file module runtime_env

e529973

Closes: ray-project#23151 Signed-off-by: Chi-Sheng Liu <[email protected]>

[Feat][Core] Implement single file module for runtime_env

78268db

Closes: ray-project#23151 Signed-off-by: Chi-Sheng Liu <[email protected]>

[Test][Core] Add tests for single file py_modules runtime_env

0a7d46f

Signed-off-by: Chi-Sheng Liu <[email protected]>

[Chore] Address minor issue comments

01bf72e

Closes: ray-project#23151 Signed-off-by: Chi-Sheng Liu <[email protected]>

[Refactor] Rename some variables to make it clear

dc77943

Closes: ray-project#23151 Signed-off-by: Chi-Sheng Liu <[email protected]>

[Test][Core] Add tests for single file module in test_runtime_env_wor…

324cfe7

…king_dir.py Closes: ray-project#23151 Signed-off-by: Chi-Sheng Liu <[email protected]>

[Refactor] Address issue comment (rename variables)

ea58122

Closes: ray-project#23151 Signed-off-by: Chi-Sheng Liu <[email protected]>

MortalHappiness force-pushed the bugfix/#23151-python-modules branch from a0d5bc2 to ea58122 Compare September 25, 2024 22:41

rynewang enabled auto-merge (squash) September 26, 2024 00:54

github-actions bot added the go add ONLY when ready to merge, run all tests label Sep 26, 2024

rynewang approved these changes Sep 26, 2024

View reviewed changes

rynewang assigned angelinalg Sep 26, 2024

rynewang removed the triage Needs triage (eg: priority, bug/not-bug, and owning component) label Sep 26, 2024

MortalHappiness requested a review from rkooo567 September 26, 2024 23:44

angelinalg approved these changes Oct 11, 2024

View reviewed changes

rynewang merged commit 61a4220 into ray-project:master Oct 11, 2024
7 checks passed

MortalHappiness mentioned this pull request Oct 12, 2024

[Chore][Core] Address PR 47807 comments #48002

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feat][Core] Implement single file module for runtime_env #47807

[Feat][Core] Implement single file module for runtime_env #47807

MortalHappiness commented Sep 24, 2024

rynewang left a comment

MortalHappiness commented Sep 25, 2024 •

edited

Loading

MortalHappiness commented Sep 25, 2024

rynewang left a comment

rynewang commented Sep 26, 2024

angelinalg left a comment

angelinalg Oct 11, 2024

angelinalg Oct 11, 2024

angelinalg Oct 11, 2024

angelinalg Oct 11, 2024

angelinalg Oct 11, 2024

angelinalg Oct 11, 2024

angelinalg Oct 11, 2024

angelinalg Oct 11, 2024

rynewang commented Oct 11, 2024

	This function will hash the path of the file or directory,
	This function hashes the path of the file or directory,

	and if it's a file, it'll hash its content too.
	and if it's a file, then it hashes its content too.

	f"because the file could not be opened: {e}"
	f"because the file couldn't be opened: {e}"

	It'll hash the path of the file and its content to create a hash value.
	It hashes the path of the file and its content to create a hash value.

	This function will generate the name of the package by the file.
	This function generates the name of the package by the file.

	The final package name is: _ray_pkg_<HASH_VAL>.zip of this package.
	The final package name is _ray_pkg_<HASH_VAL>.zip, where <HASH_VAL> is a hash of this package.

	e.g., _ray_pkg_029f88d5ecc55e1e4d64fc6e388fd103.zip
	For example: _ray_pkg_029f88d5ecc55e1e4d64fc6e388fd103.zip

[Feat][Core] Implement single file module for runtime_env #47807

[Feat][Core] Implement single file module for runtime_env #47807

Conversation

MortalHappiness commented Sep 24, 2024

Why are these changes needed?

Related issue number

Checks

rynewang left a comment

Choose a reason for hiding this comment

MortalHappiness commented Sep 25, 2024 • edited Loading

MortalHappiness commented Sep 25, 2024

rynewang left a comment

Choose a reason for hiding this comment

rynewang commented Sep 26, 2024

angelinalg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rynewang commented Oct 11, 2024

MortalHappiness commented Sep 25, 2024 •

edited

Loading