(enh) Change occ background-job:list limit default 10->500#40042
Merged
nickvergessen merged 5 commits intonextcloud:masterfrom Nov 15, 2023
Merged
Conversation
szaimen
requested review from
a team,
ArtificialOwl,
come-nc and
nfebe
and removed request for
a team
joshtrichards
changed the title
occ background-job:list limit default 10->100
Member
|
Why just dont |
Member
Author
Yes, the parameter works. I just think It's may seem a nitpick, but it also creates more mental load for what seems no net benefit from what I can tell. |
Contributor
|
Maybe it makes sense to add a note to the output like „Showing the first X jobs“, or „Output is limited to X jobs“ to indicate that there’s a limit active. |
come-nc
approved these changes
Aug 28, 2023
Contributor
come-nc
left a comment
come-nc
left a comment
There was a problem hiding this comment.
Maybe an indication in the output if the list is complete or not would indeed be also good.
joshtrichards
force-pushed
the
jr-occ-background-job-list-limit-100
branch
from
168c57c to
42c976f
Compare
Member
Author
|
Done. Though, technically, it breaks json output format... |
come-nc
reviewed
Aug 29, 2023
Merged
mejo-
approved these changes
Nov 14, 2023
mejo-
force-pushed
the
jr-occ-background-job-list-limit-100
branch
2 times, most recently
from
34211c3 to
32bdd7f
Compare
come-nc
approved these changes
Nov 14, 2023
kesselb
approved these changes
Nov 14, 2023
mejo-
force-pushed
the
jr-occ-background-job-list-limit-100
branch
from
3f527ae to
136a199
Compare
come-nc
reviewed
Nov 14, 2023
The default limit of 10 seems too conservative. Nearly all environments >100 and most <10. At the same time, this higher default limit still remains reasonable to avoid problems in really big environments. Signed-off-by: Josh Richards <[email protected]>
Co-authored-by: Daniel <[email protected]> Signed-off-by: Jonas <[email protected]>
* Only print the comment when job list is truncated * Show the comment at the end so users actually see it * Format the comment as comment Signed-off-by: Jonas <[email protected]>
Signed-off-by: Jonas <[email protected]>
Co-authored-by: Côme Chilliet <[email protected]> Signed-off-by: Jonas <[email protected]>
mejo-
force-pushed
the
jr-occ-background-job-list-limit-100
branch
from
7fc0675 to
af4287d
Compare
Merged
nickvergessen
approved these changes
Nov 15, 2023
mejo-
changed the title
occ background-job:list limit default 10->100occ background-job:list limit default 10->500
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
I'd been wondering for awhile why not all background jobs would show up under
occ background-job:listeven though all jobs were clearly being executed. Spent an embarrassing amount of time reviewing the code on this... only to realize there was a default limit of10in the command. 🤣I understand why there is a limit (large environments) - and it can be easily overridden - but I feel 10 is bit too conservative for a default. Being this low means the job list is nearly always incomplete in the default execution mode, which makes the overriding of the default nearly a 100% certainty in day to day use.
Nearly all environments >10 and many (most?) <100 jobs. This changes the default to 100 so fewer have to feel as silly as me. 😃 And a lot more people get all their jobs shown by default. At the same time, this higher default still remains conservative enough to avoid problems in really large environments.
TODO
Checklist
[ ] Screenshots before/after for front-end changes[ ] Backports requested where applicable (ex: critical bugfixes)