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

Implementation of EOS-supported locking #94

Merged
merged 12 commits into from
Nov 14, 2022
Merged

Implementation of EOS-supported locking #94

merged 12 commits into from
Nov 14, 2022

Conversation

glpatcern
Copy link
Member

@glpatcern glpatcern commented Oct 12, 2022

Internal reference: https://its.cern.ch/jira/browse/CERNBOX-3016

Through validation tests, it turns out that rename operations (MoveRequest in CS3 API terms) must be supported regardless if a file is locked - edit: renames may still need a lock, regardless how Microsoft Office behaves. The test suite was amended to take that into account. Similarly, the wopiserver needs to be able to set an xattr on an unlocked file, but be denied if the file is locked and the lock does not match. This was incorporated as well in the test suite.

Validation tests:

  • local interface (CI)
  • xroot interface
    - [ ] cs3 interface
  • MS WOPI validator tests on top of xroot
    - [ ] MS WOPI validator tests on top of cs3

Update: as renames were not changed, and as right now the CS3 interface on top of Reva master does not pass all the tests, we're not executing those tests. In a separate PR we will integrate such tests on the CI, with a custom GitHub runner similar to the Reva repo.

@glpatcern glpatcern self-assigned this Oct 12, 2022
@glpatcern glpatcern requested a review from javfg as a code owner October 12, 2022 10:16
@glpatcern glpatcern marked this pull request as draft October 12, 2022 10:16
@glpatcern glpatcern force-pushed the eoslock branch 16 times, most recently from 7ccab6e to 7422cbd Compare October 18, 2022 15:19
@glpatcern glpatcern marked this pull request as ready for review October 18, 2022 15:22
@glpatcern glpatcern changed the title WIP: initial implementation of EOS-supported locking Implementation of EOS-supported locking Oct 18, 2022
@glpatcern glpatcern marked this pull request as draft October 19, 2022 07:34
@glpatcern glpatcern force-pushed the eoslock branch 2 times, most recently from f947e85 to 5950b91 Compare October 19, 2022 09:33
@glpatcern glpatcern marked this pull request as ready for review October 19, 2022 09:39
@glpatcern glpatcern force-pushed the eoslock branch 4 times, most recently from 7c99f82 to 311b9b5 Compare October 19, 2022 12:47
@glpatcern
Copy link
Member Author

@micbar would you be able to test this PR against Reva edge, and run the MS WOPI validator on top?

A priori the changes and fixes to the CS3 interface are minimal, yet there is an important improvement, that is single-file shares are properly supported now (i.e. SaveAs is disabled - that was already in master - and PutRelative now fails with HTTP 501 as expected by the MS WOPI validator tests), and that is possibly useful for the OCIS GA release.

@micbar
Copy link
Member

micbar commented Nov 8, 2022

@glpatcern We need the Wopi Validator in the CI of this repo, running on every Pull Request.

See https://drone.owncloud.com/owncloud/wopi/2177/9/10 as an example.

You are using GH actions in this repo, so you would need to adapt the test code a bit.

@glpatcern
Copy link
Member Author

@glpatcern We need the Wopi Validator in the CI of this repo, running on every Pull Request.

@micbar I agree this would be highly beneficial, but a precondition is to run the WOPI tests against a live Reva daemon (#57).

See https://drone.owncloud.com/owncloud/wopi/2177/9/10 as an example.

I don't have access to your drone, maybe you can enable my GH handle?

@glpatcern glpatcern force-pushed the eoslock branch 3 times, most recently from b6050ae to d03465d Compare November 11, 2022 13:00
@glpatcern glpatcern merged commit 17285e6 into master Nov 14, 2022
@glpatcern glpatcern deleted the eoslock branch November 14, 2022 11:51
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