-
Notifications
You must be signed in to change notification settings - Fork 23
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
Project access refactoring #481
Conversation
fda516c
to
cf1b025
Compare
2f8d677
to
a901752
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this, it really does clean up the API. Few minor comments from my side only.
geti_sdk/geti.py
Outdated
|
||
:param project_name: Name of the project to retrieve | ||
:param project_id: ID of the project to retrieve. If not specified, the | ||
project with name `project_name` will be retrieved. | ||
:param project: Project object to update. If not specified, the project with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence is not finished. I'm not sure about the behaviour, does the project
override the project_id
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just looked at the ProjectClient.get_project
method, the behaviour is clear now. Could you add a brief description of the parameter order here? Mainly because users may use the Geti
object as the entrypoint to the API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the docstring, please see if the explanations are clearer now
Signed-off-by: Igor-Davidyuk <[email protected]>
Signed-off-by: Igor-Davidyuk <[email protected]>
Signed-off-by: Igor-Davidyuk <[email protected]>
Signed-off-by: Igor-Davidyuk <[email protected]>
Signed-off-by: Igor-Davidyuk <[email protected]>
Signed-off-by: Igor-Davidyuk <[email protected]>
Signed-off-by: Igor-Davidyuk <[email protected]>
Signed-off-by: Igor-Davidyuk <[email protected]>
Signed-off-by: Igor-Davidyuk <[email protected]>
Signed-off-by: Igor-Davidyuk <[email protected]>
Signed-off-by: Igor-Davidyuk <[email protected]>
Signed-off-by: Igor-Davidyuk <[email protected]>
Signed-off-by: Igor-Davidyuk <[email protected]>
Co-authored-by: Ludo Cornelissen <[email protected]>
Co-authored-by: Ludo Cornelissen <[email protected]>
Signed-off-by: Igor-Davidyuk <[email protected]>
35b1a40
to
2faeac2
Compare
This PR adds guardrails to the user workflow that should streamline the experience of working with identically named projects.
The
get_project
now may be called with one of [project_name, project_id, project_object].Fetching a project by name now has an unraveled logic scheme and is THE place where the user can determine the correct
project_id
.All the other methods now accept a project object as an argument instead of a project_name which may be potentially problematic.