Skip to content
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.

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

Hostpath volume #60

Merged
merged 4 commits into from
May 22, 2017
Merged

Conversation

Yancey1989
Copy link
Collaborator

Fixed #51

#DATACENTERS = {
# ...
# "dc1":{
# "type": "hostpath",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe fstype is better?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@@ -270,6 +270,15 @@
# "admin_key": "/certs/admin.secret"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should contain type in this example.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

type => fstype
Done.

))
if dc == name:
if cfg["type"] == "cephfs":
volumes.append(CephFSVolume(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Check if monitors_addr user secret exists. Return an error code when parameters not found.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These parameters are in settings.py, I think we can check the configurations at the start-up phase of the cloud server. And maybe implement this feature in another PR is suitable.

@@ -0,0 +1,4 @@
from cephfs_volume import CephFSVolume
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems package volume is too small to exist. We can have:

def get_volume_config(**kwargs):
    __check_parameters(kwargs)
    tmpl = __get_template(kwargs["type"])
    return json.loads(__render(tmpl, kwargs))

Copy link
Collaborator Author

@Yancey1989 Yancey1989 May 22, 2017

Choose a reason for hiding this comment

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

Good point!I have use templates instead of package volume.Thanks a lot!
Done.

}

def __render(tmpl, **kwargs):
tmpl.replace("$NAME", "sfsdf")
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line is not needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Copy link
Collaborator

@typhoonzero typhoonzero left a comment

Choose a reason for hiding this comment

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

LGTM

@Yancey1989 Yancey1989 merged commit 484aa27 into PaddlePaddle:develop May 22, 2017
@Yancey1989 Yancey1989 deleted the hostpath_volume branch May 22, 2017 11:42
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.

2 participants