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(core): allow per dataset data directory #3027

Merged
merged 12 commits into from Jul 25, 2022

Conversation

Panaetius
Copy link
Member

closes #2977

Adds a --datadir option to renku dataset create, renku dataset add --create and renku dataset import)

@Panaetius Panaetius force-pushed the feature/2977-datadir-per-dataset branch from e172452 to 5d7c914 Compare July 18, 2022 15:28
olevski
olevski previously approved these changes Jul 19, 2022
Copy link
Member

@olevski olevski left a comment

Choose a reason for hiding this comment

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

I see that that assert statement was there already. So there is perhaps a good reason to leave it in. I just wanted to point it out. Everything else looks good.

tests/cli/test_integration_datasets.py Show resolved Hide resolved
renku/core/dataset/providers/local.py Outdated Show resolved Hide resolved
Copy link
Contributor

@m-alisafaee m-alisafaee left a comment

Choose a reason for hiding this comment

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

Thanks Ralf! This looks great! Please see my comments.

renku/core/dataset/dataset.py Outdated Show resolved Hide resolved
renku/core/dataset/providers/local.py Outdated Show resolved Hide resolved
renku/core/dataset/providers/renku.py Outdated Show resolved Hide resolved
renku/domain_model/dataset.py Outdated Show resolved Hide resolved
renku/domain_model/dataset.py Outdated Show resolved Hide resolved
renku/domain_model/dataset.py Outdated Show resolved Hide resolved
@m-alisafaee
Copy link
Contributor

We should also display dataset's data dir in renku dataset ls and renku dataset show.

@Panaetius Panaetius force-pushed the feature/2977-datadir-per-dataset branch from a539df9 to 233c747 Compare July 20, 2022 08:38
@Panaetius
Copy link
Member Author

Don't merge until a new release is made

m-alisafaee
m-alisafaee previously approved these changes Jul 20, 2022
Copy link
Contributor

@m-alisafaee m-alisafaee left a comment

Choose a reason for hiding this comment

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

Tests are failing due to minimum version check. OK otherwise.

@Panaetius
Copy link
Member Author

Tests are failing due to minimum version check. OK otherwise.

I was worried that might be the case. I think last time when we set it to 1.2.0, we were already on 1.2.4 or something like that. Maybe we should change the minimum version to be exclusive, i.e. 1.5.0 would mean "Version must be higher than 1.5.0", and then 1.5.0.dev25+gd4dfa1ce might pass. Though I doubt it.

@Panaetius Panaetius force-pushed the feature/2977-datadir-per-dataset branch 3 times, most recently from abdae49 to 06de6b6 Compare July 21, 2022 11:02
@Panaetius Panaetius force-pushed the feature/2977-datadir-per-dataset branch from 06de6b6 to fec519a Compare July 21, 2022 11:11
@Panaetius Panaetius force-pushed the feature/2977-datadir-per-dataset branch from 2b87168 to e04d40f Compare July 22, 2022 12:46
@Panaetius Panaetius force-pushed the feature/2977-datadir-per-dataset branch from 1e144d2 to 8761c3c Compare July 25, 2022 08:06
Copy link
Contributor

@m-alisafaee m-alisafaee left a comment

Choose a reason for hiding this comment

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

🚀

@Panaetius Panaetius merged commit aecc180 into develop Jul 25, 2022
@Panaetius Panaetius deleted the feature/2977-datadir-per-dataset branch July 25, 2022 11:03
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.

Change datadir to be per dataset instead of per project
3 participants