Skip to content

[serve] Migrate parse_uri from _private to _common#64371

Open
ShuChenLin wants to merge 6 commits into
ray-project:masterfrom
ShuChenLin:migrate-runtime-env-uri-common
Open

[serve] Migrate parse_uri from _private to _common#64371
ShuChenLin wants to merge 6 commits into
ray-project:masterfrom
ShuChenLin:migrate-runtime-env-uri-common

Conversation

@ShuChenLin

@ShuChenLin ShuChenLin commented Jun 26, 2026

Copy link
Copy Markdown

Description

This PR continues the migration from ray._private to ray._common. Ray Serve previously imported parse_uri from ray._private.runtime_env.packaging. This PR moves parse_uri and its minimal dependencies into _common, updates Serve to import from _common, and removes the private parse_uri implementation from runtime-env packaging.

Changes:

  1. ray._common.runtime_env_uri

    • Adds Protocol, is_path and parse_uri for runtime-env URI parsing.
    • Keeps the implementation lightweight and parse-only.
  2. Private runtime-env updates

    • Removes parse_uri from ray._private.runtime_env.packaging.
    • Updates remaining runtime-env callers to import parse_uri from ray._common.runtime_env_uri.
    • Reuses the common Protocol enum in ray._private.runtime_env.protocol, while keeping private download_remote_uri behavior attached there.
  3. Ray Serve import update

    • Updates python/ray/serve/schema.py to import parse_uri from ray._common.runtime_env_uri instead of ray._private.runtime_env.packaging.
  4. Tests

    • Adds python/ray/_common/tests/test_runtime_env_uri.py.
    • Mirrors the existing TestParseUri test cases from python/ray/tests/test_runtime_env_packaging.py.
    • Registers the new test in python/ray/_common/tests/BUILD.bazel.

Related issues

Related to #53478.

Additional information

  • This PR only migrates the parse_uri function. It does not attempt to migrate or refactor the broader runtime_env package.

@ShuChenLin ShuChenLin requested review from a team, MengjinYan, dayshah and edoakes as code owners June 26, 2026 09:26

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces cross-platform path and URI utilities under ray/_common, including a new is_path helper and a parse_uri function with associated unit tests, and updates ray/serve/schema.py to use the new module. Feedback on the changes includes importing urllib.parse directly to prevent potential runtime errors and refactoring the dynamically constructed Protocol enum into a standard Python class to improve readability, type safety, and IDE autocompletion.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread python/ray/_common/path_utils.py Outdated
@@ -0,0 +1,32 @@
import pathlib
import urllib

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Importing urllib does not automatically expose the urllib.parse submodule in Python. Relying on other modules to have imported it first can lead to a runtime AttributeError: module 'urllib' has no attribute 'parse'. Please import urllib.parse directly.

Suggested change
import urllib
import urllib.parse

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment thread python/ray/_common/runtime_env_uri.py Outdated
Comment on lines +7 to +48
_PROTOCOLS = (
# For packages dynamically uploaded and managed by the GCS.
"gcs",
# For conda environments installed locally on each node.
"conda",
# For pip environments installed locally on each node.
"pip",
# For uv environments installed locally on each node.
"uv",
# Remote http path, assumes everything packed in one zip file.
"http",
# Remote https path, assumes everything packed in one zip file.
"https",
# Remote s3 path, assumes everything packed in one zip file.
"s3",
# Remote google storage path, assumes everything packed in one zip file.
"gs",
# Remote azure blob storage path, assumes everything packed in one zip file.
"azure",
# Remote Azure Blob File System Secure path, assumes everything packed in one zip file.
"abfss",
# File storage path, assumes everything packed in one zip file.
"file",
)

_REMOTE_PROTOCOLS = ("http", "https", "s3", "gs", "azure", "abfss", "file")

Protocol = enum.Enum(
"Protocol",
{protocol.upper(): protocol for protocol in _PROTOCOLS},
)


@classmethod
def _remote_protocols(cls):
# Returns a list of protocols that support remote storage.
# These protocols should only be used with paths that end in
# ".zip", ".whl", ".tar.gz", or ".tgz".
return [cls[protocol.upper()] for protocol in _REMOTE_PROTOCOLS]


Protocol.remote_protocols = _remote_protocols

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Instead of dynamically constructing the Protocol Enum using the functional API and monkey-patching a classmethod, it is much cleaner, more readable, and type-safe to define it as a standard Python class. This also enables better IDE autocompletion and static type checking.

_REMOTE_PROTOCOLS = ("http", "https", "s3", "gs", "azure", "abfss", "file")


class Protocol(enum.Enum):
    # For packages dynamically uploaded and managed by the GCS.
    GCS = "gcs"
    # For conda environments installed locally on each node.
    CONDA = "conda"
    # For pip environments installed locally on each node.
    PIP = "pip"
    # For uv environments installed locally on each node.
    UV = "uv"
    # Remote http path, assumes everything packed in one zip file.
    HTTP = "http"
    # Remote https path, assumes everything packed in one zip file.
    HTTPS = "https"
    # Remote s3 path, assumes everything packed in one zip file.
    S3 = "s3"
    # Remote google storage path, assumes everything packed in one zip file.
    GS = "gs"
    # Remote azure blob storage path, assumes everything packed in one zip file.
    AZURE = "azure"
    # Remote Azure Blob File System Secure path, assumes everything packed in one zip file.
    ABFSS = "abfss"
    # File storage path, assumes everything packed in one zip file.
    FILE = "file"

    @classmethod
    def remote_protocols(cls):
        # Returns a list of protocols that support remote storage.
        # These protocols should only be used with paths that end in
        # ".zip", ".whl", ".tar.gz", or ".tgz".
        return [cls[protocol.upper()] for protocol in _REMOTE_PROTOCOLS]

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@ray-gardener ray-gardener Bot added serve Ray Serve Related Issue core Issues that should be addressed in Ray Core community-contribution Contributed by the community labels Jun 26, 2026
@@ -19,7 +18,7 @@
)

from ray._common.logging_constants import LOGRECORD_STANDARD_ATTRS

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's drop parse_uri from ray._private.runtime_env.packaging.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed and updated the PR description.

I have updated the remaining callers in ray._private to import parse_uri from ray._common.runtime_env_uri instead.

@ShuChenLin ShuChenLin requested a review from a team as a code owner June 26, 2026 16:10
@@ -10,12 +9,11 @@
from pathlib import Path
from tempfile import TemporaryDirectory
from typing import Callable, List, Optional, Tuple
from urllib.parse import urlparse
from zipfile import ZipFile

from filelock import FileLock

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we drop is_path from path_utils?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. I removed ray._common.path_utils and inlined the path check as a private helper in runtime_env_uri.py.

Signed-off-by: ShuChenLin <ch25585.leader.tw@gmail.com>
Signed-off-by: ShuChenLin <ch25585.leader.tw@gmail.com>
Signed-off-by: ShuChenLin <ch25585.leader.tw@gmail.com>
Signed-off-by: ShuChenLin <ch25585.leader.tw@gmail.com>
Signed-off-by: ShuChenLin <ch25585.leader.tw@gmail.com>
@ShuChenLin ShuChenLin force-pushed the migrate-runtime-env-uri-common branch from e796fdf to 90f6db6 Compare June 29, 2026 05:44
Signed-off-by: ShuChenLin <ch25585.leader.tw@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution Contributed by the community core Issues that should be addressed in Ray Core serve Ray Serve Related Issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants