Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve _init_server #12019

Closed
harupy opened this issue May 16, 2024 · 2 comments · Fixed by #12076
Closed

Improve _init_server #12019

harupy opened this issue May 16, 2024 · 2 comments · Fixed by #12076
Assignees
Labels
good first issue Good for newcomers has-closing-pr This issue has a closing PR

Comments

@harupy
Copy link
Member

harupy commented May 16, 2024

Summary

Make the following change, otherwise the subprocess running the server will wait forever and hang:

diff --git a/tests/tracking/integration_test_utils.py b/tests/tracking/integration_test_utils.py
index f64ea82d1..3a1076579 100644
--- a/tests/tracking/integration_test_utils.py
+++ b/tests/tracking/integration_test_utils.py
@@ -60,13 +60,15 @@ def _init_server(backend_uri, root_artifact_uri, extra_env=None, app="mlflow.ser
             **(extra_env or {}),
         },
     ) as proc:
-        _await_server_up_or_die(server_port)
-        url = f"http://{LOCALHOST}:{server_port}"
-        _logger.info(
-            f"Launching tracking server against backend URI {backend_uri}. Server URL: {url}"
-        )
-        yield url
-        proc.terminate()
+        try:
+            _await_server_up_or_die(server_port)
+            url = f"http://{LOCALHOST}:{server_port}"
+            _logger.info(
+                f"Launching tracking server against backend URI {backend_uri}. Server URL: {url}"
+            )
+            yield url
+        finally:
+            proc.terminate()

Notes

  • Make sure to open a PR from a non-master branch.

  • Sign off the commit using the -s flag when making a commit:

    git commit -s -m "..."
             # ^^ make sure to use this
  • Include #{issue_number} (e.g. #123) in the PR description when opening a PR.

@harupy harupy added the good first issue Good for newcomers label May 16, 2024
@zhouyou9505
Copy link
Contributor

hi sir, Could you assign it to me @harupy

@harupy
Copy link
Member Author

harupy commented May 17, 2024

@zhouyou9505 Thanks! Assigned :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers has-closing-pr This issue has a closing PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants