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

[full-ci] Bump reva to include ini metadata backend #5613

Merged
merged 7 commits into from
Feb 23, 2023
Merged

Conversation

butonic
Copy link
Member

@butonic butonic commented Feb 21, 2023

bump reva to include cs3org/reva#3649

@update-docs
Copy link

update-docs bot commented Feb 21, 2023

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@butonic butonic marked this pull request as ready for review February 21, 2023 11:55
@butonic butonic changed the title [full-ci] Test ini backend [full-ci] Bump reva to include ini metadata backend Feb 21, 2023
@mmattel
Copy link
Contributor

mmattel commented Feb 21, 2023

This is for sure docs relevant. We need to discuss this.

@butonic
Copy link
Member Author

butonic commented Feb 21, 2023

This is for sure docs relevant. We need to discuss this.

We don't change defaults and if it isn't set it will also default to the existing xattrs backend. For now nothing changes. We can now however use a new metadata backend which needs more testing. For that we want to have the ini backend in ocis asap.

We still need to add a migration. When that is in place we will update the docs and explain the differences in more detail.

@butonic butonic requested review from C0rby and kobergj and removed request for C0rby February 21, 2023 13:57
@butonic butonic requested a review from kobergj February 22, 2023 13:16
Copy link
Collaborator

@kobergj kobergj left a comment

Choose a reason for hiding this comment

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

Please add description also to storage-users

services/storage-users/pkg/config/config.go Outdated Show resolved Hide resolved
services/storage-users/pkg/config/config.go Outdated Show resolved Hide resolved
@mmattel
Copy link
Contributor

mmattel commented Feb 22, 2023

Q: we have now:
STORAGE_SYSTEM_OCIS_METADATA_BACKEND and
STORAGE_USERS_OCIS_METADATA_BACKEND

Is there a reason not having a global ennvar?
And if not, one could be added starting with OCIS_xxx

butonic and others added 7 commits February 22, 2023 17:48
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
Co-authored-by: kobergj <jkoberg@owncloud.com>
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
@butonic
Copy link
Member Author

butonic commented Feb 22, 2023

Q: we have now: STORAGE_SYSTEM_OCIS_METADATA_BACKEND and STORAGE_USERS_OCIS_METADATA_BACKEND

Is there a reason not having a global ennvar? And if not, one could be added starting with OCIS_xxx

yes, the metadata option is storage specific. you could use a different metadata backend for the system storage and the users storage.

we could still add a single OCIS_DECOMPOSEDFS_METADATA_BACKEND option, but I don't think that makes it any clearer.

@butonic butonic requested a review from kobergj February 22, 2023 16:55
@sonarcloud
Copy link

sonarcloud bot commented Feb 22, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mmattel
Copy link
Contributor

mmattel commented Feb 23, 2023

yes, the metadata option is storage specific. you could use a different metadata backend for the system storage and the users storage.

we could still add a single OCIS_DECOMPOSEDFS_METADATA_BACKEND option, but I don't think that makes it any clearer.

It was not meant to replace but ADD them like we do with others.

This would finally look like:
OCIS_DECOMPOSEDFS_METADATA_BACKEND, STORAGE_SYSTEM_OCIS_METADATA_BACKEND

and

OCIS_DECOMPOSEDFS_METADATA_BACKEND, STORAGE_USERS_OCIS_METADATA_BACKEND

@butonic
Copy link
Member Author

butonic commented Feb 23, 2023

@mmattel That can be done in a subsequent PR. I'm not sure we will have the ini xattr backend for much longer.

@butonic butonic merged commit a5fb068 into master Feb 23, 2023
@delete-merged-branch delete-merged-branch bot deleted the test-ini-backend branch February 23, 2023 09:54
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.

3 participants