Conversation
Co-authored-by: Richard Liaw <rliaw@berkeley.edu>
|
I'll add unittesting for the integration tomorrow (similar to the Kubernetes test). |
| from ray.autoscaler._private.util import validate_config | ||
| from ray.autoscaler.node_provider import _get_node_provider | ||
|
|
||
| with open(config_path) as f: | ||
| new_config = yaml.safe_load(f.read()) | ||
| validate_config(new_config) | ||
| self._config = new_config |
There was a problem hiding this comment.
FYI @ericl for Tune to sync between docker containers, we leverage some of the autoscaler's private interfaces.
There was a problem hiding this comment.
Please advise if there's a better alternative
There was a problem hiding this comment.
Can you instead add validate_config to the public API (sdk.py)?
There was a problem hiding this comment.
@richardliaw @krfricke can we have a design document on this? It looks like this is a pretty complex piece of machinery here and it's concerning it is peeking into the internals of the autoscaler. This makes it likely to be broken by future changes.
|
Let's use #11122 as the API for rsync. |
| from ray.tune.integration import docker | ||
| docker.rsync = self.mock_command |
There was a problem hiding this comment.
looks good, - generally you'll want to use mock.patch instead
addressed comments, fully public api
Why are these changes needed?
Add support for syncing between docker containers in Ray Tune.
Related issue number
Replaces and closes #10800
Also, closes #4403
Checks
scripts/format.shto lint the changes in this PR.