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

Decouple spaces API from storage #215

Merged
merged 2 commits into from
Oct 25, 2023
Merged

Conversation

gmgigi96
Copy link
Member

The spaces API are currently coupled with the storage. This PR decouple them from the storage, allowing a more flexible implementation.

@gmgigi96 gmgigi96 marked this pull request as ready for review October 25, 2023 09:00
@labkode
Copy link
Member

labkode commented Oct 25, 2023

@gmgigi96 looks good to me to split the spaces and the storage api into two separate entities as the spaces sit on top storage and have functionalities on top of it.

This is a breaking change because of a new API, however it should be straighforward at the code level to change as the messages are the same.

@cs3org/owncloud-team please review.

@micbar
Copy link
Member

micbar commented Oct 25, 2023

Hmm, from my pov I consider a storage provider which has no spaces support as an invalid implementation.

@labkode
Copy link
Member

labkode commented Oct 25, 2023

@micbar reva@master does not have Spaces support and is a valid implementation :)

@gmgigi96
Copy link
Member Author

gmgigi96 commented Oct 25, 2023

Hmm, from my pov I consider a storage provider which has no spaces support as an invalid implementation.

@micbar I think where you store spaces is just a matter of implementation detail. For us is a DB, for you the storage itself. That's why this need to be separated.

@micbar
Copy link
Member

micbar commented Oct 25, 2023

@micbar reva@master does not have Spaces support and is a valid implementation :)

Correct. But as I understood you some years ago, that should only be temporary. You had the plan to converge with reva edge. Did this plan change?

@labkode labkode merged commit 3646193 into cs3org:main Oct 25, 2023
2 checks passed
@labkode
Copy link
Member

labkode commented Oct 25, 2023

@micbar plan is to converge on web only, and look at future options later.

@micbar
Copy link
Member

micbar commented Oct 25, 2023

@micbar I think where you store spaces is just a matter of implementation detail. For us is a DB, for you the storage itself. That's why this need to be separated.

I think you are missing the details. Every Stat call or every Call to the storage provider needs to implement the spaces semantics. Resources are referenced by <StorageID><SpaceID><OpaqueID>.

Reva master does only path based requests. On Reva Edge, Spaces are a basic architectural concept of total decentralization. Reva Master has the concept that there is a global path which is the same for everybody. These two concepts are different in its core.

Maybe I am the only one who is concerned about diverging too much.

@butonic @dragotin @kobergj @dragonchaser

labkode pushed a commit that referenced this pull request Dec 19, 2023
labkode added a commit that referenced this pull request Dec 19, 2023
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.

None yet

3 participants