Skip to content

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

Merged
merged 13 commits into from
Feb 18, 2021

Conversation

AgatheG
Copy link
Contributor

@AgatheG AgatheG commented Feb 11, 2021

ch59800

A PR is opened to add the API in the doc: https://github.com/dataiku/dss-doc/pull/458

  • Move and rename DSSProjectDeployerProject#import_bundle (to DSSProjectDeployer#upload_bundle)
    • Do not pass design node info as params anymore
    • Pass the published key as a param (to match what we do in the UI)
  • DSSProjectDeployerDeploymentSettings: add a property bundle_id
  • Add a DSSProjectDeployerProjectStatus#get_deployments method
  • Add wrapper classes for infra (DSSProjectDeployerInfraStatus/DSSAPIDeployerInfraStatus)
  • Add a DSSProject#publish_bundle method

Agathe Guillemot added 3 commits February 11, 2021 11:52
@AgatheG AgatheG added this to the 9.0 milestone Feb 11, 2021
@AgatheG AgatheG self-assigned this Feb 11, 2021
@shortcut-integration
Copy link

This pull request has been linked to Clubhouse Story #59800: Feedback on APIs for PDPL.

@AgatheG
Copy link
Contributor Author

AgatheG commented Feb 11, 2021

The only one I did not do from the CH card is:

To know if a project already exists in PDPL, add a DSSProjectDeployer#get_project(project_key): returns None if not found or a DSSProjectDeployerProject object if found

As the user will get an error trying to use any method from the created DSSProjectDeployerProject anyway.

@AgatheG AgatheG requested a review from instanceofme February 12, 2021 10:43
@AgatheG AgatheG requested a review from mhoarau February 15, 2021 14:51
Copy link
Contributor

@mhoarau mhoarau left a 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.

@mhoarau
Copy link
Contributor

mhoarau commented Feb 16, 2021

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.

Agathe Guillemot added 5 commits February 16, 2021 12:32
Copy link
Contributor

@mhoarau mhoarau left a 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):
Copy link
Contributor

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?

Copy link
Contributor Author

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?

@instanceofme instanceofme merged commit a52cf37 into master Feb 18, 2021
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