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

Unpin setproctitile #45640

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Unpin setproctitile #45640

wants to merge 2 commits into from

Conversation

mattip
Copy link
Contributor

@mattip mattip commented May 30, 2024

Why are these changes needed?

Related issue number

Fixes #40289, which was reverted in #45539. Also adapt tests to run locally by changing the pattern matching for the case where the tests are run in a virtualenv, and the process reporting mechanism gets the full path to the original python. Additiionally, on macos that original python may be Python with a capital P so loosen the pattern matcing.

#40289 was reverted due to breaking a test due to dvarrazzo/py-setproctitle#118. Again, allow the pattern matching to pass even when a single character is missing from the end of the command.

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

subprocess.check_call([sys.executable, v2_driver])
env = os.environ.copy()
# Make sure `lib` can be loaded
env.pop("PYTHONSAFEPATH", None)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is important since the lib.py file must be loaded from the local directory. In order to run tests "in place" without building ray, safe path isolation can be used, but then inside the test lib will not be found.

@mattip
Copy link
Contributor Author

mattip commented May 30, 2024

In order to run tests locally on macos, I needed to set RAY_ENABLE_WINDOWS_OR_OSX_CLUSTER=1.

In order to run tests without actually building ray, I could set PYTHONSAFEPATH=1 which will avoid adding '.' to the sys.path and import ray from site-packages, but then I needed to remove that from the env for some tests

@mattip
Copy link
Contributor Author

mattip commented May 30, 2024

Are the four failing linux tests known to be flaky or is the failure due to updating setproctitle?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant