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

[Tune] Unrevert "Add more comprehensive support for remote_checkpoint_dir w/ url params (#32479)" #32576

Merged
merged 7 commits into from
Feb 16, 2023

Conversation

justinvyu
Copy link
Contributor

@justinvyu justinvyu commented Feb 15, 2023

Why are these changes needed?

This reverts the revert made here #32571.

This PR fixes two suites of tests that were failing:

  1. workflows tests were failing due to the pinned moto version being too old.
  2. test_syncer errors were caused by reusing the same bucket name between tests.

This PR also fixes the restore path for URIs with params, which was missing from the previous PR. The e2e sync test has been updated to also test restoration.

See changes from new commits only here: new commits

Related issue number

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

…r remote `upload_dir` w/ endpoint and params (ray-project#32479)" (ray-project#32571)"

This reverts commit 350fb13.
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
@justinvyu
Copy link
Contributor Author

@krfricke Before merging, I want to run workflows tests - how do I kick these off?

@krfricke krfricke merged commit abdb7c7 into ray-project:master Feb 16, 2023
@krfricke
Copy link
Contributor

Ah sorry, that was a quick merge...

usually it would have been [all_tests] or changing something in a workflows file.

Well, now it will run on master, let's see if everything works

@justinvyu
Copy link
Contributor Author

Ah ok, hopefully it is ok!

@xwjiang2010
Copy link
Contributor

Thanks folks! Will keep an eye out!

edoakes pushed a commit to edoakes/ray that referenced this pull request Mar 22, 2023
…_dir w/ url params (ray-project#32479)" (ray-project#32576)

This reverts the revert made here ray-project#32571.

This PR fixes two suites of tests that were failing:
1. workflows tests were failing due to the pinned `moto` version being too old.
2. `test_syncer` errors were caused by reusing the same bucket name between tests.

This PR also fixes the `restore` path for URIs with params, which was missing from the previous PR. The e2e sync test has been updated to also test restoration.

Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
peytondmurray pushed a commit to peytondmurray/ray that referenced this pull request Mar 22, 2023
…_dir w/ url params (ray-project#32479)" (ray-project#32576)

This reverts the revert made here ray-project#32571.

This PR fixes two suites of tests that were failing:
1. workflows tests were failing due to the pinned `moto` version being too old.
2. `test_syncer` errors were caused by reusing the same bucket name between tests.

This PR also fixes the `restore` path for URIs with params, which was missing from the previous PR. The e2e sync test has been updated to also test restoration.

Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
elliottower pushed a commit to elliottower/ray that referenced this pull request Apr 22, 2023
…_dir w/ url params (ray-project#32479)" (ray-project#32576)

This reverts the revert made here ray-project#32571.

This PR fixes two suites of tests that were failing:
1. workflows tests were failing due to the pinned `moto` version being too old.
2. `test_syncer` errors were caused by reusing the same bucket name between tests.

This PR also fixes the `restore` path for URIs with params, which was missing from the previous PR. The e2e sync test has been updated to also test restoration.

Signed-off-by: Justin Yu <justinvyu@berkeley.edu>
Signed-off-by: elliottower <elliot@elliottower.com>
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.

3 participants