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 more upload session filters #7933

Merged
merged 5 commits into from
Jan 26, 2024
Merged

add more upload session filters #7933

merged 5 commits into from
Jan 26, 2024

Conversation

butonic
Copy link
Member

@butonic butonic commented Dec 11, 2023

This PR deprecates the ocis storage-users uploads list command:

NAME:
   ocis storage-users uploads list - Print a list of all incomplete uploads (deprecated, use sessions)

USAGE:
   ocis storage-users uploads list [command options] [arguments...]

OPTIONS:
   --help, -h  show help

It also introduces the ocis storage-users uploads sessions command:

NAME:
   ocis storage-users uploads sessions - Print a list of upload sessions

USAGE:
   ocis storage-users uploads sessions [command options] [arguments...]

OPTIONS:
   --id value      filter sessions by upload session id
   --processing    filter sessions by processing status
   --expired       filter sessions by expired status
   --output value  output format to use (can be 'plain' or 'json',  experimental) (default: plain)
   --help, -h      show help

In oc10 occ we also added cli flags to allow rendiring output as json. I marked that as experimental, let me know what you think.

The storageprovider only has a boolean processing flag. Before we can expose processing step outcomes via the upload session interface we need to change the persistence of upload sessions. Currently, it lives in the node metadata, we should move it to the upload session so every uploed session can track its own processing status.

The filter currently only allows an expired true/false flag. I don't think we need a full timestamp filter, yet.

Should we add a --purge option to the session command? it would allow purging specific upload sessions ... or multiple based on filter. or is that too much of a shotgun?

How do we restart postprocessing? should we add a --restart option and then trigger postprocessing?

Let me know what you think!

Copy link

update-docs bot commented Dec 11, 2023

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

0.0% 0.0% Coverage
16.1% 16.1% Duplication

@butonic
Copy link
Member Author

butonic commented Jan 23, 2024

  • add command to restart all unprocessed uploads

@micbar
Copy link
Contributor

micbar commented Jan 23, 2024

@butonic please add dev docs about the new command.

@butonic
Copy link
Member Author

butonic commented Jan 23, 2024

@kobergj Hm, how do we actually determine if an upload session is stuck in postrocessing? Only postprocessing service really knows. Currently, the storage provider derives the session IsProcessing() response by calling node.IsProcessing() so all uploads to the same file share the same processing state ...

Only the postrocessing service knows the state. The storage provider will emit an UploadReady{Failed: true}. I'll need to check what that does. If all is well the postprocessing service can iterate over all UploadReady events with failed true and just restart them? But how do we differentiate that from an upload that aborted because of a policy or virus found? In case of outcome PPOutcomeAbort or PPOutcomeDelete an UploadReady{Failed: true} event is emitted. No way to distinguish them.

@kobergj
Copy link
Collaborator

kobergj commented Jan 23, 2024

It does nothing, but it should probably. 😄 Let's pair on it tomorrow!

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic butonic force-pushed the cli-upload-session-options branch from 4e264b1 to 1811f3a Compare January 24, 2024 16:23
@butonic butonic marked this pull request as ready for review January 24, 2024 16:23
@butonic
Copy link
Member Author

butonic commented Jan 24, 2024

the command to restart upload sessions needs to go into the postprocessing service, so this PR is complete.

Copy link
Collaborator

@kobergj kobergj left a comment

Choose a reason for hiding this comment

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

I can't believe you commented your exported funcs! Finally the linter can be happy even without me 😄

Code looks good in general, just some nitpicks as always 👍

services/storage-users/pkg/command/uploads.go Show resolved Hide resolved
services/storage-users/pkg/command/uploads.go Outdated Show resolved Hide resolved
services/storage-users/pkg/command/uploads.go Show resolved Hide resolved
@kobergj
Copy link
Collaborator

kobergj commented Jan 24, 2024

the command to restart upload sessions needs to go into the postprocessing service, so this PR is complete.

Already working on this, but needs a round trip through reva: cs3org/reva#4477

ocis PR: #8287

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@butonic butonic requested a review from kobergj January 25, 2024 12:21
Copy link

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

3 New issues
0 Security Hotspots
0.0% Coverage on New Code
13.9% Duplication on New Code

See analysis details on SonarCloud

Copy link
Collaborator

@kobergj kobergj left a comment

Choose a reason for hiding this comment

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

LGTM

@butonic butonic merged commit 3ed0424 into master Jan 26, 2024
3 checks passed
@delete-merged-branch delete-merged-branch bot deleted the cli-upload-session-options branch January 26, 2024 08:21
ownclouders pushed a commit that referenced this pull request Jan 26, 2024
* add more upload session filters

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>

* default descriptions

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>

* update readme

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>

* use tablewriter, --json flag

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>

* update readme

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>

---------

Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
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