Skip to content

Conversation

@barakmich
Copy link
Contributor

Way better than #13310 -- no extra client-classes, simply control placement groups by sending the commands as tasks

Related issue number

Closes #13147

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

Change-Id: I1af5219a2aa057603cd0eca534dda0f1c9ef067b
Change-Id: I6677eaa35688eb73aa26fd35c4432769cb35f8f4
@barakmich barakmich requested a review from rkooo567 February 11, 2021 18:09
@rkooo567 rkooo567 self-assigned this Feb 11, 2021
Copy link
Contributor

@rkooo567 rkooo567 left a comment

Choose a reason for hiding this comment

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

LGTM.

@rkooo567
Copy link
Contributor

rkooo567 commented Feb 12, 2021

Btw, are we testing this somewhere? (not so familiar with client code base)

@ericl
Copy link
Contributor

ericl commented Feb 16, 2021

Seems to be breaking placement group tests (and we should add some client tests).

@ericl ericl added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Feb 16, 2021
@ericl ericl self-assigned this Feb 16, 2021
@ericl
Copy link
Contributor

ericl commented Mar 8, 2021

Closing for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

@author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support placement groups in Ray client

3 participants