[DO NOT MERGE] Debug Proxy#63506
Conversation
… URIs (ray-project#62813)" This reverts commit c6e7001. Signed-off-by: davik <davik@anyscale.com>
There was a problem hiding this comment.
Code Review
This pull request removes support for .tar.gz, .tgz, and .tar.bz2 archive formats within Ray's runtime_env, standardizing on .zip and .whl files. The changes include removing the tarfile dependency, simplifying packaging and URI parsing logic, and updating documentation and tests to reflect the new restrictions. Feedback focused on correcting documentation typos and incomplete command examples, as well as improving code readability in the delete_package function by avoiding variable shadowing and leveraging existing helper functions.
|
|
||
| - ``working_dir`` (str): Specifies the working directory for the Ray workers. This must either be (1) a local existing directory with total size at most 500 MiB, (2) a local existing archive file (``.zip``, ``.tar.gz``, or ``.tgz``) with total uncompressed size at most 500 MiB (Note: ``excludes`` has no effect), or (3) a URI to a remotely-stored archive (``.zip``, ``.tar.gz``, or ``.tgz``) containing the working directory for your job (no file size limit is enforced by Ray). See :ref:`remote-uris` for details. | ||
| The specified directory is downloaded to each node on the cluster, and Ray workers start in their node's copy of this directory. | ||
| - ``working_dir`` (str): Specifies the working directory for the Ray workers. This must either be (1) an local existing directory with total size at most 500 MiB, (2) a local existing zipped file with total unzipped size at most 500 MiB (Note: ``excludes`` has no effect), or (3) a URI to a remotely-stored zip file containing the working directory for your job (no file size limit is enforced by Ray). See :ref:`remote-uris` for details. |
There was a problem hiding this comment.
Typo: "an local" should be "a local".
| - ``working_dir`` (str): Specifies the working directory for the Ray workers. This must either be (1) an local existing directory with total size at most 500 MiB, (2) a local existing zipped file with total unzipped size at most 500 MiB (Note: ``excludes`` has no effect), or (3) a URI to a remotely-stored zip file containing the working directory for your job (no file size limit is enforced by Ray). See :ref:`remote-uris` for details. | |
| - ``working_dir`` (str): Specifies the working directory for the Ray workers. This must either be (1) a local existing directory with total size at most 500 MiB, (2) a local existing zipped file with total unzipped size at most 500 MiB (Note: ``excludes`` has no effect), or (3) a URI to a remotely-stored zip file containing the working directory for your job (no file size limit is enforced by Ray). See :ref:`remote-uris` for details. |
| Check for hidden files and metadata directories in zipped dependencies. | ||
| You can inspect a zip file's contents by running the ``zipinfo -1 zip_file_name.zip`` command in the Terminal. | ||
| Some zipping methods can cause hidden files or metadata directories to appear in the zip file at the top level. | ||
| To avoid this, use the ``zip -r`` command directly on the directory you want to compress from its parent's directory. For example, if you have a directory structure such as: ``a/b`` and you want to compress ``b``, issue the ``zip -r b`` command from the directory ``a.`` |
There was a problem hiding this comment.
The command zip -r b is incomplete; it requires an output filename (e.g., zip -r b.zip b). Additionally, there is a typo in the directory name reference where the period is included inside the backticks (a.).
| To avoid this, use the ``zip -r`` command directly on the directory you want to compress from its parent's directory. For example, if you have a directory structure such as: ``a/b`` and you want to compress ``b``, issue the ``zip -r b`` command from the directory ``a.`` | |
| To avoid this, use the ``zip -r`` command directly on the directory you want to compress from its parent's directory. For example, if you have a directory structure such as: ``a/b`` and you want to compress ``b``, issue the ``zip -r b.zip b`` command from the directory ``a``. |
| path = Path(_get_local_path(base_directory, pkg_uri)) | ||
| with FileLock(str(path) + ".lock"): | ||
| path = path.with_suffix("") | ||
| if path.exists(): | ||
| if path.is_dir() and not path.is_symlink(): | ||
| shutil.rmtree(str(path)) | ||
| else: | ||
| local_dir.unlink() | ||
| path.unlink() | ||
| deleted = True |
There was a problem hiding this comment.
The variable path is reassigned from the package file path to the directory path, which reduces code clarity. It is better to use distinct variable names and leverage the existing get_local_dir_from_uri helper function for consistency and maintainability.
| path = Path(_get_local_path(base_directory, pkg_uri)) | |
| with FileLock(str(path) + ".lock"): | |
| path = path.with_suffix("") | |
| if path.exists(): | |
| if path.is_dir() and not path.is_symlink(): | |
| shutil.rmtree(str(path)) | |
| else: | |
| local_dir.unlink() | |
| path.unlink() | |
| deleted = True | |
| pkg_path = Path(_get_local_path(base_directory, pkg_uri)) | |
| with FileLock(str(pkg_path) + ".lock"): | |
| local_dir = get_local_dir_from_uri(pkg_uri, base_directory) | |
| if local_dir.exists(): | |
| if local_dir.is_dir() and not local_dir.is_symlink(): | |
| shutil.rmtree(str(local_dir)) | |
| else: | |
| local_dir.unlink() | |
| deleted = True |
Description
Related issues
Additional information