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

[core] Add possibility to pass ObjectRef to get_task #41611

Closed
wants to merge 1 commit into from

Conversation

max-509
Copy link
Contributor

@max-509 max-509 commented Dec 5, 2023

Why are these changes needed?

Prior to these changes to pass task id to get_task() function user needs to call object_ref.task_id().hex() which is quite painful. This PR adds possibility to pass ObjectRef to get_task() function.

Related issue number

Closes #41307

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

Signed-off-by: max-509 <123456vershinin@gmail.com>
@hongchaodeng hongchaodeng self-requested a review February 22, 2024 00:12
@hongchaodeng
Copy link
Member

@max-509 Would you update the PR: 1. fix lint check; 2. rebase master?

assert f_task["node_id"] == node_id
assert f_task["actor_id"] is None

assert put_task is None
Copy link
Member

Choose a reason for hiding this comment

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

It makes no sense to test this put_task. Please remove it.

@@ -2417,6 +2417,35 @@ def verify():
print(list_tasks())


def test_get_task_object_ref(shutdown_only):
Copy link
Member

Choose a reason for hiding this comment

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

We should combine the test of objectRef into test_list_get_tasks()

@hongchaodeng
Copy link
Member

Since we haven't heard from you, I have cherry pick your commit and make another PR with the commented changes: #43507

Let me know if you still want to work on this. Thanks for your contribution and good will!

@anyscalesam anyscalesam added stale The issue is stale. It will be closed within 7 days unless there are further conversation core Issues that should be addressed in Ray Core triage Needs triage (eg: priority, bug/not-bug, and owning component) labels Feb 28, 2024
@max-509
Copy link
Contributor Author

max-509 commented Feb 29, 2024

@hongchaodeng Hello! I think you can close my PR and finish this task in your PR.

@stale stale bot removed the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Issues that should be addressed in Ray Core triage Needs triage (eg: priority, bug/not-bug, and owning component)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Core] Ray state API get_task() should accept ObjectRef
3 participants