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

move root mount setup to mountproviders #31266

Merged
merged 8 commits into from
Mar 4, 2022
Merged

Conversation

icewind1991
Copy link
Member

@icewind1991 icewind1991 commented Feb 18, 2022

Removes special casing for the root storage from the filesystem setup logic

Requires:

Part of #31265

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Feb 18, 2022
@icewind1991 icewind1991 added this to the Nextcloud 24 milestone Feb 18, 2022
@icewind1991 icewind1991 mentioned this pull request Feb 18, 2022
8 tasks
@icewind1991 icewind1991 force-pushed the root-setup-mountprovider branch 4 times, most recently from c0732ab to 17e2bdd Compare February 23, 2022 19:36
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
…torage

Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
…g part files

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
Copy link
Member Author

Added ci setup with objectstore (multibucket) as root storage and fixed some failures when running with objectstore

@skjnldsv skjnldsv requested review from a team and nickvergessen and removed request for a team March 1, 2022 14:24
@icewind1991 icewind1991 mentioned this pull request Mar 3, 2022
1 task
Copy link
Member

@CarlSchwan CarlSchwan left a comment

Choose a reason for hiding this comment

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

Looks good 👍 and tests are happy

@@ -0,0 +1,70 @@
name: S3 primary storage
Copy link
Member

Choose a reason for hiding this comment

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

hurrah for more tests!

Copy link
Member

Choose a reason for hiding this comment

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

is this backportable separately?

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

Code looks fine 👍

It does seem to also contain changes that perhaps could be extracted to separate PRs to make them backportable if they solve bigger issues ?

// the entire operation will be safe

try {
$this->acquireLock(ILockingProvider::LOCK_EXCLUSIVE);
Copy link
Member

Choose a reason for hiding this comment

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

is this directly related to this PR or is this a fix that we could use separately ?

@@ -0,0 +1,70 @@
name: S3 primary storage
Copy link
Member

Choose a reason for hiding this comment

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

is this backportable separately?

@icewind1991
Copy link
Member Author

I'll look into backporting the primary storage ci+fixes

@PVince81
Copy link
Member

PVince81 commented Mar 4, 2022

I'll look into backporting the primary storage ci+fixes

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants