-
Notifications
You must be signed in to change notification settings - Fork 108
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
[FEAT] Automatically use Ray Runner if Ray is initialized #2282
base: main
Are you sure you want to change the base?
Conversation
29b4964
to
eb69621
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a few comments, but otherwise looks great!
A few more thoughts:
- The usage of the lock seems correct to me 👍
- Have you tested this manually, or maybe could we write some basic tests for this behavior?
- In
set_runner_py
, the docstrings say "this is the default behavior", that should be changed.
eb69621
to
e7ab3d2
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2282 +/- ##
=======================================
Coverage ? 79.12%
=======================================
Files ? 471
Lines ? 54404
Branches ? 0
=======================================
Hits ? 43049
Misses ? 11355
Partials ? 0
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
ray.init(...)
before running any Daft queryingDAFT_RAY_ADDRESS
environment variable so that we can centralize on the normalRAY_ADDRESS
behaviorThis PR ensures the following behavior:
daft.context.set_runner_ray/py
, this overrides all behaviorDAFT_RUNNER
environment variable forRAY
/PY
RAY
PY
Ray connection detection, on driver vs on worker:
Warning if set_runner_ray is called with an address after Ray is already initialized: