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

fix: send dataset current image url when updating the dataset (#1861) #1887

Merged
merged 3 commits into from
Jun 10, 2022

Conversation

andre-code
Copy link
Contributor

@andre-code andre-code commented Jun 7, 2022

PR to send the url of the current image when updating the dataset so as not to delete the existing image.
Currently the endpoint for updating a dataset requests the url of the current image from the dataset to keep that image, otherwise it deletes the image. This is a workaround for this case, but the API should not delete the image if it is not included in the dataset object when updating a dataset.

closes #1861

/deploy #persist

@andre-code andre-code temporarily deployed to renku-ci-ui-1887 June 7, 2022 12:31 Inactive
@RenkuBot
Copy link
Contributor

RenkuBot commented Jun 7, 2022

You can access the deployment of this PR at https://renku-ci-ui-1887.dev.renku.ch

@andre-code andre-code temporarily deployed to renku-ci-ui-1887 June 7, 2022 13:21 Inactive
@andre-code andre-code marked this pull request as ready for review June 7, 2022 13:26
@andre-code andre-code requested a review from a team as a code owner June 7, 2022 13:26
@andre-code andre-code temporarily deployed to renku-ci-ui-1887 June 9, 2022 09:45 Inactive
@andre-code andre-code temporarily deployed to renku-ci-ui-1887 June 9, 2022 14:19 Inactive
Copy link
Contributor

@ciyer ciyer left a comment

Choose a reason for hiding this comment

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

The code changes look good, but when testing I encountered some problems, and maybe we can fix them as part of this issue, since it is so far not too big.

  1. I get an error when I try to create a dataset that includes an image
    image

(on dev, I can create a dataset and supply an image in the form -- create succeeds, but the image is not is not actually added 🤷 )

  1. Once I have added an image to the form, there is no way to remove the image

@andre-code andre-code deployed to renku-ci-ui-1887 June 10, 2022 10:31 Active
@andre-code
Copy link
Contributor Author

@ciyer Thanks for the review and testing, indeed there was something to fix in the creation of the dataset. Fixed in the last commit.
about the delete option we can include it when this is ready SwissDataScienceCenter/renku-python#2938

Copy link
Contributor

@ciyer ciyer left a comment

Choose a reason for hiding this comment

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

Works great now! 🙏 Do not worry about the acceptance tests, those are fixed in another PR. You can merge even if they do not pass.

@andre-code andre-code merged commit 70ba40e into master Jun 10, 2022
@andre-code andre-code deleted the 1861-fix-update-image-dataset branch June 10, 2022 14:50
@RenkuBot
Copy link
Contributor

Tearing down the temporary RenkuLab deplyoment for this PR.

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.

The dataset image should not be deleted when updating the dataset
3 participants