-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
[Core] Fix worker crash when getting actor name from runtime context #45194
Conversation
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.
How about updating runtime_context.py
instead of core_worker.cc
to keep the behavior consistent with other functions such as get_task_id
and get_actor_id
?
To elaborate,
def get_actor_name(self) -> Optional[str]:
"""Get the current actor name of this worker.
This shouldn't be used in a driver process.
The name is in string format.
Returns:
The current actor name of this worker.
If a current worker is an actor, and
if actor name doesn't exist, it returns an empty string.
If a current worker is not an actor, it returns None.
"""
# only worker mode has actor_id
if self.worker.mode != ray._private.worker.WORKER_MODE:
logger.warning(
"This method is only available when the process is a "
f"worker. Current mode: {self.worker.mode}"
)
return None <---- Add this line
actor_id = self.worker.actor_id
return self.worker.actor_name if not actor_id.is_nil() else None
You can refer to get_task_id as an example.
@kevin85421 that good ! |
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.
Would you mind adding a test in test_runtime_context.py
? Btw, you can add -s
in your git commit
command to avoid the DCO failure in CI.
@kevin85421 ok |
Signed-off-by: 982945902 <982945902@qq.com>
Signed-off-by: 982945902 <982945902@qq.com>
@@ -1,4 +1,4 @@ | |||
<a id="try-anyscale-href" href="https://forms.gle/DFin3AzKEDB19uXZ6"> | |||
<a id="try-anyscale-href" href="https://www.anyscale.com"> |
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.
this line change is unrelated? could you rebase properly?
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.
ok
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.
Would you mind rebasing with the master branch? |
@kevin85421 not mind, how need i do |
Take a look at your commit history. It seems to be that you don't sync with the master branch correctly. You may not need to rebase with the master branch, but you need to fix your commit history. |
Signed-off-by: chuanzhisongshu <49055103+982945902@users.noreply.github.com>
Hi @982945902, would you mind fixing the CI failure? |
@kevin85421 It seems that some of the test cases timed out, causing the use cases to fail, how should I re-triggered the C test |
I re-triggered CI pipeline multiple times for this PR. It always fails. Could you try to reproduce in your local env? |
@kevin85421 These test cases have bugs, and someone merged and fixed them two days ago |
1 similar comment
@kevin85421 These test cases have bugs, and someone merged and fixed them two days ago |
Would you mind rebasing with the master branch? Thanks! |
@kevin85421 not mind, how need i do |
Sync your branch with the upstream master branch. |
@kevin85421 done |
…ay-project#45194) Currently calling get_runtime_context().get_actor_name() from driver will crash. Instead of crashing, this PR returns None in this case. Signed-off-by: 982945902 <982945902@qq.com> Co-authored-by: Huaiwei Sun <scottsun94@gmail.com> Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
…ay-project#45194) Currently calling get_runtime_context().get_actor_name() from driver will crash. Instead of crashing, this PR returns None in this case. Signed-off-by: 982945902 <982945902@qq.com> Co-authored-by: Huaiwei Sun <scottsun94@gmail.com> Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>
…ay-project#45194) Currently calling get_runtime_context().get_actor_name() from driver will crash. Instead of crashing, this PR returns None in this case. Signed-off-by: 982945902 <982945902@qq.com> Co-authored-by: Huaiwei Sun <scottsun94@gmail.com>
…ay-project#45194) Currently calling get_runtime_context().get_actor_name() from driver will crash. Instead of crashing, this PR returns None in this case. Signed-off-by: 982945902 <982945902@qq.com> Co-authored-by: Huaiwei Sun <scottsun94@gmail.com> Signed-off-by: gchurch <gabe1church@gmail.com>
Why are these changes needed?
Currently calling get_runtime_context().get_actor_name() from driver will crash. Instead of crashing, this PR returns None in this case.
Related issue number
Closes #45174
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.