-
Notifications
You must be signed in to change notification settings - Fork 26
Project deployer: update some PDPL methods #122
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
Conversation
Rename Adapt to new Public API implem Move inside DSSProjectDeployer
This pull request has been linked to Clubhouse Story #59800: Feedback on APIs for PDPL. |
The only one I did not do from the CH card is:
As the user will get an error trying to use any method from the created |
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 did not get a chance to play with the API but I have a few comments.
I have just noticed that I was late on the doc branch, so maybe my comments about the doc are wrong but I have an error while trying to build the doc so I can't check immediately. |
Add DSSAPIDeployerServiceStatus#get_deployments
Add utils#CallableStr for backward compatibility
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 still did not play with the APIs but LGTM. I just have one remaining question.
@@ -302,8 +364,19 @@ def get_raw(self): | |||
""" | |||
return self.settings | |||
|
|||
@property | |||
def bundle_id(self): |
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.
So we don't want to have a setter in the end?
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.
Clément suggested to use a property here as we do for most ids. Users can still set the field with bundle_id = new_bundle_id
, do you want me to add a comment to make it more explicit?
ch59800
A PR is opened to add the API in the doc: https://github.com/dataiku/dss-doc/pull/458
DSSProjectDeployerProject#import_bundle
(toDSSProjectDeployer#upload_bundle
)DSSProjectDeployerDeploymentSettings
: add a propertybundle_id
DSSProjectDeployerProjectStatus#get_deployments
methodDSSProjectDeployerInfraStatus
/DSSAPIDeployerInfraStatus
)(now merged)DSSAPIDeployerInfra#get_status
/DSSAPIDeployerInfra#get_status
corresponding public API calls are implemented in https://github.com/dataiku/dip/pull/12012DSSProject#publish_bundle
methodcorresponding public API call in https://github.com/dataiku/dip/pull/12012(now merged)