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

add spaceID to resourceID #174

Merged
merged 1 commit into from
Jun 21, 2022
Merged

add spaceID to resourceID #174

merged 1 commit into from
Jun 21, 2022

Conversation

micbar
Copy link
Member

@micbar micbar commented Jun 21, 2022

Description

To correctly address storage spaces in reva, we need another id property on the resourceID.

This change is not breaking and should be compatible for the master and the edge branches.

  1. masterbranch.

We do not need to change anything

  1. edge branch

Currently the storageID attribute is used for a concatenated <storageproviderID>$<spaceID>

With this change, we will be able to separate them and have a clean implementation back.

@labkode @glpatcern I will also update the cs3apis on the master branch to keep everything on the same version of the apis.

@micbar micbar requested a review from labkode as a code owner June 21, 2022 12:37
@labkode
Copy link
Member

labkode commented Jun 21, 2022

@micbar I agree with @kobergj and @glpatcern that this field MUST be an optional field for time being.
Apart from ownCloud, no one is using the SpacesAPI for their reference implementation.

@kobergj
Copy link
Contributor

kobergj commented Jun 21, 2022

@micbar @butonic @labkode @glpatcern How about we add a new OPTIONAL StorageProviderID and use the StorageID as SpaceID in ocis? That is basically the way ocis does it now too anyways.

@micbar
Copy link
Member Author

micbar commented Jun 21, 2022

@micbar I agree with @kobergj and @glpatcern that this field MUST be an optional field for time being.
Apart from ownCloud, no one is using the SpacesAPI for their reference implementation.

That surprises me.

I thought we have an agreement that spaces are the first class citizens in the API.

@micbar
Copy link
Member Author

micbar commented Jun 21, 2022

@micbar @butonic @labkode @glpatcern How about we add a new OPTIONAL StorageProviderID and use the StorageID as SpaceID in ocis? That is basically the way ocis does it now too anyways.

That is what ocis does today and IMO not clean. See the original description of storage_id which means exactly the provider.

@micbar
Copy link
Member Author

micbar commented Jun 21, 2022

@labkode @glpatcern I changed the REQUIRED flag to OPTIONAL. Not a blocker for me.

Please let us continue here.

@glpatcern
Copy link
Member

@micbar I think it's the best option at this point in time, leaving the door open to both approaches. Will merge.

@glpatcern glpatcern merged commit 726aedb into cs3org:main Jun 21, 2022
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.

5 participants