Skip to content

Revert pull retry#19068

Merged
scv119 merged 4 commits intoray-project:masterfrom
rkooo567:revert-pull-retry
Oct 4, 2021
Merged

Revert pull retry#19068
scv119 merged 4 commits intoray-project:masterfrom
rkooo567:revert-pull-retry

Conversation

@rkooo567
Copy link
Copy Markdown
Contributor

@rkooo567 rkooo567 commented Oct 4, 2021 •
edited by mwtian
Loading

Why are these changes needed?

It looks like after these PRs are merged, the dask on ray test starts timeout (this test never times out for last 1 month).

Related issue number

#19042

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

@rkooo567
Copy link
Copy Markdown
Contributor Author

rkooo567 commented Oct 4, 2021

PRs that are reverted;

d12e35c
56debfc

mwtian
mwtian approved these changes Oct 4, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@mwtian mwtian left a comment

Choose a reason for hiding this comment

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

Investigating in #19042

@rkooo567
Copy link
Copy Markdown
Contributor Author

rkooo567 commented Oct 4, 2021

Btw I am not 100% sure yet if this is the root cause, so let’s merge it and see for a couple days. If you can repro it somehow that’d be great too

@mwtian
Copy link
Copy Markdown
Member

mwtian commented Oct 4, 2021

Sounds good, let's see if time out will happen again. Currently it is a bit strange that no timeout happens for very similar tests.

@rkooo567
Copy link
Copy Markdown
Contributor Author

rkooo567 commented Oct 4, 2021

Btw there was a lint failure https://buildkite.com/ray-project/ray-builders-pr/builds/15147#97e71d2b-9614-4830-8ff6-1e7b0ed87192, that I couldn't reproduce in my local env. Do you know why this happens? I've seen this a lot recently.

@rkooo567
Copy link
Copy Markdown
Contributor Author

rkooo567 commented Oct 4, 2021

@scv119 can you merge the PR after tests are done?

@mwtian
Copy link
Copy Markdown
Member

mwtian commented Oct 4, 2021

Btw there was a lint failure https://buildkite.com/ray-project/ray-builders-pr/builds/15147#97e71d2b-9614-4830-8ff6-1e7b0ed87192, that I couldn't reproduce in my local env. Do you know why this happens? I've seen this a lot recently.

This is from clang-tidy. #19055 should run clang-tidy locally and give better messages.

@scv119
Copy link
Copy Markdown
Contributor

scv119 commented Oct 4, 2021

//python/ray/tests:test_client_reconnect is failing on windows, shouldn't be related.

@scv119 scv119 merged commit 83cb992 into ray-project:master Oct 4, 2021
mwtian pushed a commit to mwtian/ray that referenced this pull request Oct 7, 2021
Revert "Revert pull retry (ray-project#19068)"
fcf0a61 
This reverts commit 83cb992.
mwtian pushed a commit that referenced this pull request Oct 7, 2021
Revert "Revert pull retry (#19068)"
a603251 
This reverts commit 83cb992.
mwtian pushed a commit that referenced this pull request Oct 7, 2021
Revert "Revert pull retry (#19068)"
5866f5a 
This reverts commit 83cb992.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mwtian mwtian mwtian approved these changes

Assignees

@scv119 scv119

@mwtian mwtian

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rkooo567 @mwtian @scv119