Skip to content

Check if the provider is external before getting the config. #1743

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.

Sign up for GitHub

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

Jump to bottom
Merged
richardliaw merged 5 commits into from
Mar 23, 2018
Merged

Check if the provider is external before getting the config. #1743

richardliaw merged 5 commits into from
Mar 23, 2018

Conversation

@barrachri
Copy link
Contributor

@barrachri barrachri commented Mar 18, 2018 •
edited by richardliaw
Loading

Fixes #1737

@AmplabJenkins
Copy link

AmplabJenkins commented Mar 18, 2018

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/4391/
Test FAILed.

@AmplabJenkins
Copy link

AmplabJenkins commented Mar 18, 2018

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/4393/
Test PASSed.

@barrachri barrachri mentioned this pull request Mar 19, 2018
Closed
@richardliaw
Copy link
Contributor

richardliaw commented Mar 20, 2018

Pushed a fix for docker as it was getting snagged

@richardliaw
Copy link
Contributor

richardliaw commented Mar 20, 2018

Do you have an example yaml where this works?

richardliaw
richardliaw requested changes Mar 20, 2018
View reviewed changes
Copy link
Contributor

@richardliaw richardliaw left a comment

Choose a reason for hiding this comment

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

some form of test/example would be good to make sure this codepath actually works!

@AmplabJenkins
Copy link

AmplabJenkins commented Mar 20, 2018

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/4418/
Test PASSed.

@barrachri
Copy link
Contributor Author

barrachri commented Mar 20, 2018

Do you have an example yaml where this works?

I am adding a external_config_example.yml for this.

some form of test/example would be good to make sure this codepath actually works!

Testing that the path actually works or more like end to end test?

@richardliaw
Copy link
Contributor

richardliaw commented Mar 20, 2018

Something like a external_config_example.yml would probably be fine; ideally we could test this end to end by treating the AWS provider classes as external modules, but probably not too urgent at this point.

@ericl
Copy link
Contributor

ericl commented Mar 23, 2018

I'm fine without an example file -- I think only very technical users would use this feature, so it may even be better to leave it out as it might be confusing.

@ericl
Copy link
Contributor

ericl commented Mar 23, 2018 

    type: external
    module: ray.autoscaler.aws.node_provider.AWSNodeProvider

does something reasonable (no AWS autoconfigure, but that's expected).

@richardliaw richardliaw merged commit 13b3df9 into ray-project:master Mar 23, 2018
@richardliaw
Copy link
Contributor

richardliaw commented Mar 23, 2018

Test error was valgrind - seems unrelated.

@barrachri barrachri deleted the fix_1737 branch March 23, 2018 07:30
royf added a commit to royf/ray that referenced this pull request Apr 22, 2018
@royf
Merge commit 'f69cbd35d4e86f2a3c2ace875aaf8166edb69f5d' into softq
30565ab 
* commit 'f69cbd35d4e86f2a3c2ace875aaf8166edb69f5d': (64 commits)
  Bump version to 0.4.0. (ray-project#1745)
  Fix monitor.py bottleneck by removing excess Redis queries. (ray-project#1786)
  Convert the ObjectTable implementation to a Log (ray-project#1779)
  Acquire worker lock when importing actor. (ray-project#1783)
  Introduce a log interface for the new GCS (ray-project#1771)
  [tune] Fix linting error (ray-project#1777)
  [tune] Added pbt with keras on cifar10 dataset example (ray-project#1729)
  Add a GCS table for the xray task flatbuffer (ray-project#1775)
  [tune] Change tune resource request syntax to be less confusing (ray-project#1764)
  Remove from X import Y convention in RLlib ES. (ray-project#1774)
  Check if the provider is external before getting the config. (ray-project#1743)
  Request and cancel notifications in the new GCS API (ray-project#1758)
  Fix resource bookkeeping for blocked actor methods. (ray-project#1766)
  Fix bug when connecting another driver in local case. (ray-project#1760)
  Define string prefixes for all tables in the new GCS API (ray-project#1755)
  [rllib] Update RLlib to work with new actor scheduling behavior (ray-project#1754)
  Redirect output of all processes by default. (ray-project#1752)
  Add API for getting total cluster resources. (ray-project#1736)
  Always send actor creation tasks to the global scheduler. (ray-project#1757)
  Print error when actor takes too long to start, and refactor error me… (ray-project#1747)
  ...

# Conflicts:
#	python/ray/rllib/__init__.py
#	python/ray/rllib/dqn/dqn.py
#	python/ray/rllib/dqn/dqn_evaluator.py
#	python/ray/rllib/dqn/dqn_replay_evaluator.py
#	python/ray/rllib/optimizers/__init__.py
#	python/ray/rllib/tuned_examples/pong-dqn.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericl ericl ericl approved these changes

@richardliaw richardliaw richardliaw approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@barrachri @AmplabJenkins @richardliaw @ericl