Skip to content

Project folder backend object (dev prj~1)#40

Merged
spicquenot merged 13 commits intorelease/5.2from
feature/dss520-project-folder
May 24, 2019
Merged

Project folder backend object (dev prj~1)#40
spicquenot merged 13 commits intorelease/5.2from
feature/dss520-project-folder

Conversation

Now the path is now /f1/f2 instead of /f1/f2/
Copy link
Contributor

@cstenac cstenac left a comment

Choose a reason for hiding this comment

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

Small things

else:
return DSSProjectFolder(self.client, parent)

def get_children(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency, this method shoudl be called list_something rather than get_something.

To avoid doubts about whether this includes projects or not, I'd call it list_child_folders

:returns list: A list of :class:`dataikuapi.dss.projectfolders.DSSProjectFolder` to interact with its children
"""
children = self.client._perform_json("GET", "/project-folders/%s" % self.project_folder_id).get("children", [])
ret = []
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be a bit more Pythonic with a list comprehension :)

:returns list: A list of :class:`dataikuapi.dss.project.DSSProject` to interact with its projects
"""
project_keys = self.client._perform_json("GET", "/project-folders/%s" % self.project_folder_id).get("projectKeys", [])
ret = []
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

########################################################
def delete(self):
"""
Delete the project folder
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: add what will happen of current children (projects and folders)

"""
return self.settings["permissions"]

def set_permissions(self, permissions):
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we return directly, do we really need a set_ call ? I don't think we do it for other similar cases

@spicquenot spicquenot requested a review from cstenac May 6, 2019 10:15
@spicquenot spicquenot requested review from cstenac and removed request for cstenac May 7, 2019 13:14
@adescamps adescamps changed the base branch from master to release/5.2 May 16, 2019 16:21
@spicquenot spicquenot merged commit 6c50654 into release/5.2 May 24, 2019
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