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] Remove unused runtime env callback #49485

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

Conversation

dentiny
Copy link
Contributor

@dentiny dentiny commented Dec 29, 2024

I don't see the callback within Deleter used anywhere.

@dentiny dentiny added the go add ONLY when ready to merge, run all tests label Dec 29, 2024
@dentiny dentiny requested a review from a team as a code owner December 29, 2024 08:04
@dentiny dentiny force-pushed the hjiang/remove-unused-callback-runtime-env branch from b8034c8 to db7869c Compare December 29, 2024 08:05
@rynewang
Copy link
Contributor

cpp tests build failure

[2024-12-29T12:19:12Z] //:gcs_actor_manager_export_event_test FAILED TO BUILD
  | [2024-12-29T12:19:12Z] //:gcs_actor_manager_test FAILED TO BUILD
  | [2024-12-29T12:19:12Z] //:gcs_autoscaler_state_manager_test FAILED TO BUILD

do we actually use those callbacks in unit tests?

@dentiny
Copy link
Contributor Author

dentiny commented Dec 30, 2024

do we actually use those callbacks in unit tests?

No, they're unused; I should fix the compilation in the latest commit.

@rynewang rynewang enabled auto-merge (squash) December 30, 2024 19:17
@github-actions github-actions bot disabled auto-merge December 30, 2024 22:02
@rynewang rynewang enabled auto-merge (squash) December 30, 2024 23:33
@github-actions github-actions bot disabled auto-merge December 31, 2024 01:39
@jjyao
Copy link
Collaborator

jjyao commented Dec 31, 2024

//python/ray/tests:test_runtime_env_working_dir_3 failure might be real (I didn't see if failed in your other PRs)

@dentiny
Copy link
Contributor Author

dentiny commented Dec 31, 2024

//python/ray/tests:test_runtime_env_working_dir_3 failure might be real (I didn't see if failed in your other PRs)

I still think this PR is a no-op change, let me merge and run again...

@jjyao
Copy link
Collaborator

jjyao commented Dec 31, 2024

Consistent test failure so it might be due to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go add ONLY when ready to merge, run all tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants