Skip to content

Commit 4c73845

Browse files
authored
[Serve] Fix serve deploy not getting current directory added to sys.path (#43899)
Re: #43214 There is a core change that stopped adding script directory and current directory for different conditions. This accidentally stopped adding current directory when it's coming from the dashboard where in that case we only want to stop adding the script directory. This PR fixes the issue and allow serve deploy to continue working with the current directory. --------- Signed-off-by: Gene Su <e870252314@gmail.com>
1 parent 6e03772 commit 4c73845

File tree

5 files changed

+82
-4
lines changed

5 files changed

+82
-4
lines changed

python/ray/_private/worker.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2348,7 +2348,8 @@ def connect(
23482348
# Formally, the local path(s) should be added when all of these are true:
23492349
# (1) there's no working_dir (or code search path should be in working_dir),
23502350
# (2) it's not interactive mode, (there's no script file in interactive mode),
2351-
# (3) it's not in dashboard,
2351+
# (3) it's not in dashboard (should only skip script location but still append
2352+
# current directory),
23522353
# (4) it's not client mode, (handled by client code)
23532354
# (5) the driver is at the same node (machine) as the worker.
23542355
#
@@ -2358,8 +2359,6 @@ def connect(
23582359
[
23592360
job_config._runtime_env_has_working_dir(),
23602361
interactive_mode,
2361-
namespace
2362-
and namespace == ray_constants.RAY_INTERNAL_DASHBOARD_NAMESPACE,
23632362
job_config._client_job,
23642363
]
23652364
):
@@ -2370,7 +2369,13 @@ def connect(
23702369
# see https://peps.python.org/pep-0338/),
23712370
# then we shouldn't add it to the workers.
23722371
if script_directory in sys.path:
2373-
code_paths.add(script_directory)
2372+
# If user code contains names like `utils`, it will be conflicting with
2373+
# the `utils` module in the dashboard module. We should skip adding
2374+
# script directory for dashboard clients and should have the
2375+
# current directory available for loading user code.
2376+
# See: https://github.com/anyscale/product/issues/20746
2377+
if namespace != ray_constants.RAY_INTERNAL_DASHBOARD_NAMESPACE:
2378+
code_paths.add(script_directory)
23742379
current_directory = os.path.abspath(os.path.curdir)
23752380
code_paths.add(current_directory)
23762381
if len(code_paths) != 0:

python/ray/serve/tests/conftest.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,35 @@ def ray_start_stop():
151151
)
152152

153153

154+
@pytest.fixture(scope="function")
155+
def ray_start_stop_in_specific_directory(request):
156+
original_working_dir = os.getcwd()
157+
158+
# Change working directory so Ray will start in the requested directory.
159+
new_working_dir = request.param
160+
os.chdir(new_working_dir)
161+
print(f"\nChanged working directory to {new_working_dir}\n")
162+
163+
subprocess.check_output(["ray", "start", "--head"])
164+
wait_for_condition(
165+
lambda: requests.get("http://localhost:52365/api/ray/version").status_code
166+
== 200,
167+
timeout=15,
168+
)
169+
try:
170+
yield
171+
finally:
172+
# Change the directory back to the original one.
173+
os.chdir(original_working_dir)
174+
print(f"\nChanged working directory back to {original_working_dir}\n")
175+
176+
subprocess.check_output(["ray", "stop", "--force"])
177+
wait_for_condition(
178+
check_ray_stop,
179+
timeout=15,
180+
)
181+
182+
154183
@pytest.fixture
155184
def ray_instance(request):
156185
"""Starts and stops a Ray instance for this test.

python/ray/serve/tests/test_cli.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -700,5 +700,34 @@ def test_deploy_from_import_path(ray_start_stop):
700700
)
701701

702702

703+
@pytest.mark.skipif(sys.platform == "win32", reason="File path incorrect on Windows.")
704+
@pytest.mark.parametrize(
705+
"ray_start_stop_in_specific_directory",
706+
[
707+
os.path.join(os.path.dirname(__file__), "test_config_files"),
708+
],
709+
indirect=True,
710+
)
711+
def test_deploy_with_access_to_current_directory(ray_start_stop_in_specific_directory):
712+
"""Test serve deploy using modules in the current directory succeeds.
713+
714+
There was an issue where dashboard client doesn't add the current directory to
715+
the sys.path and failed to deploy a Serve app defined in the directory. This
716+
test ensures that files in the current directory can be accessed and deployed.
717+
718+
See: https://github.com/ray-project/ray/issues/43889
719+
"""
720+
# Deploy Serve application with a config in the current directory.
721+
subprocess.check_output(["serve", "deploy", "use_current_working_directory.yaml"])
722+
723+
# Ensure serve deploy eventually succeeds.
724+
def check_deploy_successfully():
725+
status_response = subprocess.check_output(["serve", "status"])
726+
assert b"RUNNING" in status_response
727+
return True
728+
729+
wait_for_condition(check_deploy_successfully, timeout=5)
730+
731+
703732
if __name__ == "__main__":
704733
sys.exit(pytest.main(["-v", "-s", __file__]))
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
from ray import serve
2+
3+
4+
@serve.deployment
5+
def f():
6+
return "hi"
7+
8+
9+
app = f.bind()
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
applications:
2+
- name: app1
3+
route_prefix: /
4+
import_path: use_current_working_directory:app
5+
deployments:
6+
- name: f

0 commit comments

Comments
 (0)