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

feat(dataset): import dataset at specific tags #2926

Merged
merged 6 commits into from Jun 10, 2022

Conversation

m-alisafaee
Copy link
Contributor

@m-alisafaee m-alisafaee commented May 24, 2022

Description

Allows specifying a dataset tag when importing a Renku dataset. It then imports the specified version (files and metadata) instead of the latest version.

Fixes #2904
Fixes #2910

@m-alisafaee m-alisafaee force-pushed the 2904-import-dataset-at-tag branch 2 times, most recently from e5ee508 to 104b0d5 Compare June 1, 2022 19:24
@m-alisafaee m-alisafaee force-pushed the 2904-import-dataset-at-tag branch 5 times, most recently from d58bd5d to 7c7b5a3 Compare June 3, 2022 15:23
@m-alisafaee m-alisafaee marked this pull request as ready for review June 8, 2022 07:13
@m-alisafaee m-alisafaee requested a review from a team as a code owner June 8, 2022 07:13
Copy link
Member

@Panaetius Panaetius left a comment

Choose a reason for hiding this comment

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

Thank you for removing attrs!

I think the whole dataset add/import logic has grown a bit organically over the years, and it's become a bit hard to follow in parts, with many edge cases and different conditions. I wonder if at some point we should refactor it, with dedicated classes for different behaviors. Similar to how we reworked the template logic.
That might make it easier to understand and maintain.

renku/core/dataset/providers/olos.py Outdated Show resolved Hide resolved
renku/core/dataset/providers/renku.py Show resolved Hide resolved
renku/core/util/metadata.py Outdated Show resolved Hide resolved
renku/infrastructure/repository.py Outdated Show resolved Hide resolved
renku/infrastructure/repository.py Show resolved Hide resolved
renku/infrastructure/repository.py Show resolved Hide resolved
renku/ui/cli/dataset.py Show resolved Hide resolved
renku/ui/service/serializers/datasets.py Show resolved Hide resolved
@m-alisafaee
Copy link
Contributor Author

I think the whole dataset add/import logic has grown a bit organically over the years, and it's become a bit hard to follow in parts, with many edge cases and different conditions. I wonder if at some point we should refactor it, with dedicated classes for different behaviors. Similar to how we reworked the template logic. That might make it easier to understand and maintain.

Totally agreed! I believe a good time for this is when we want to implement new dataset features like adding pointers from S3.

Panaetius
Panaetius previously approved these changes Jun 10, 2022
Copy link
Member

@Panaetius Panaetius left a comment

Choose a reason for hiding this comment

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

Thank you

renku/infrastructure/repository.py Show resolved Hide resolved
@m-alisafaee m-alisafaee merged commit c948a77 into develop Jun 10, 2022
@m-alisafaee m-alisafaee deleted the 2904-import-dataset-at-tag branch June 10, 2022 11:30
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.

renku dataset unlink not working for imported datasets Allow importing datasets at a specific tag
2 participants