Skip to content

Fix missing test data files dependency in rllib/BUILD#26520

Closed
Riatre wants to merge 2 commits intoray-project:masterfrom
Riatre:t2
Closed

Fix missing test data files dependency in rllib/BUILD#26520
Riatre wants to merge 2 commits intoray-project:masterfrom
Riatre:t2

Conversation

@Riatre
Copy link
Contributor

@Riatre Riatre commented Jul 13, 2022

Why are these changes needed?

See #26334 and #26517 for context.

Once this is in, it should be good to roll-forwrad again.

Related issue number

Checks

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

@Riatre
Copy link
Contributor Author

Riatre commented Jul 13, 2022

To clarify, this PR should fix master, but it does not conflict with the rollback one (#26517) and I plan to roll-forward again with this change merged if the rollback happens. Please go ahead with the rollback if it unblocks you faster.

Also, is there a good way to make sure all RLLib tests will be run for a pull request, so I can make sure it works in later roll-forward attempt? I missed this in #26334 because I didn't touch anything under rllib/ and ci/pipeline/determine_tests_to_run.py decided that RAY_CI_RLLIB_DIRECTLY_AFFECTED=0...

@pcmoritz
Copy link
Contributor

Thanks for fixing this! Before we merge the new roll-forward PR, let's have one CI run where you just set all the variables in https://github.com/ray-project/ray/blob/master/ci/pipeline/determine_tests_to_run.py#L257 to 1 (you can do this in a temporary commit in the PR) to make sure we caught all such problems that can happen in advance :)

See ray-project#26334 and ray-project#26517 for context.

Once this is in, it should be good to roll-forwrad again.
@Riatre Riatre closed this Jul 15, 2022
@Riatre Riatre deleted the t2 branch July 15, 2022 16:02
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.

2 participants