enable users to list all spaces #2692
Conversation
C0rby
requested review from
kulmann,
pascalwengerter and
lookacat
as code owners
C0rby
force-pushed
the
list-all-spaces
branch
from
9ca7362
to
db7eb43
Compare
💥 Acceptance tests Core-API-Tests-ocis-storage-3 failed. The build is cancelled... |
C0rby
force-pushed
the
list-all-spaces
branch
from
db7eb43
to
2986265
Compare
|
Kudos, SonarCloud Quality Gate passed! |
| } | ||
| value, err := json.Marshal(permissions) | ||
| if err != nil { | ||
| errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) |
There was a problem hiding this comment.
this will be a good spot to add a trace and set the state to error. There should be one in the context if tracing is enabled, and use the default traceprovider (which writes to nil) if none is configured, it's safe code, and we will benefit from the trace :D we should get in the habit of doing so I guess
|
should we merge this as it is? doesn't it need a Reva counterpart that reads the opaque? |
should be this: cs3org/reva#2207 |
| if err == nil { | ||
| permissions[settingsSvc.ListAllSpacesPermissionName] = struct{}{} | ||
| } | ||
| value, err := json.Marshal(permissions) |
There was a problem hiding this comment.
Also, why not using a plain text encoder? Since you're setting the map to be of capacity 1, I imagine for the purposes of this only and not with scaling in mind. IIRC we discussed it was a hack and could easily use a plain text encoder and save some bytes 🤷 . The encoded json stikl takes up 22 bytes (with an example title). See this playground
Nice! missed it, didn't search merged ones. Thanks, then my last review comment is invalid since it is already merged on reva! |
|
Ah damn. sorry merged too early. I should've read your comments first... 🙈 |
Description
Added a new permission
list-all-spacesand a new endpoint to list all spaces.Motivation and Context
Some users should be able to list all spaces so they can maintain them. (Update quota, etc.)
How Has This Been Tested?
Types of changes
Checklist: