Skip to content

[projects] Wrap ProjectDefinition in a class#5654

Merged
pcmoritz merged 11 commits intoray-project:masterfrom
stephanie-wang:project-definition
Sep 8, 2019
Merged

[projects] Wrap ProjectDefinition in a class#5654
pcmoritz merged 11 commits intoray-project:masterfrom
stephanie-wang:project-definition

Conversation

@stephanie-wang
Copy link
Copy Markdown
Contributor

Why are these changes needed?

Wraps some commonly needed project config values in one class. Also consolidates some helper functions.

Checks

@AmplabJenkins
Copy link
Copy Markdown

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


def cluster_yaml(self):
"""Return the project's cluster configuration filename.
"""
Copy link
Copy Markdown
Contributor

@richardliaw richardliaw Sep 7, 2019

Choose a reason for hiding this comment

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

this should be same line as above
"""Return the project's cluster configuration filename."""

"""Return the project's working directory on a cluster session.
"""
directory = os.path.join("~", self.config["name"])
if not directory.endswith("/"):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit, you can do os.path.join("~", self.config["name"], ""), or generally os.path.join(path, "") for forcing path to be a directory

@pcmoritz
Copy link
Copy Markdown
Contributor

pcmoritz commented Sep 7, 2019

Looks great, there is a few test failures from the refactor :)

@AmplabJenkins
Copy link
Copy Markdown

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

@AmplabJenkins
Copy link
Copy Markdown

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

Copy link
Copy Markdown
Contributor

@pcmoritz pcmoritz left a comment

Choose a reason for hiding this comment

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

Great change, thanks :)

@pcmoritz pcmoritz merged commit cb7102f into ray-project:master Sep 8, 2019
@pcmoritz pcmoritz deleted the project-definition branch September 8, 2019 01:30
@AmplabJenkins
Copy link
Copy Markdown

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

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.

4 participants