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

occ news:updater:job exits with code 2 if last update was too long ago #2590

Merged
merged 7 commits into from
Feb 21, 2024

Conversation

mortee
Copy link
Contributor

@mortee mortee commented Feb 15, 2024

Summary

  • Added ability to access Config to lib/Command/Updater/Job.php
  • Calculate and display time elapsed since last job execution
  • Acquire updateInterval config value
  • Calculate the tolerance thershold the same way as in src/components/AdminSettings.vue
  • Check to see if more time has passed
  • Change exit code if so

@SMillerDev
Copy link
Contributor

I'm not sure what the goal of this is.

@mortee
Copy link
Contributor Author

mortee commented Feb 16, 2024

Of course, when it comes to the News admin page indicating there's something wrong with the updater job, I need to take action. I need to make this status available outside of the visual feedback of the admin page. By being able to extract this from the occ console, I can make the appropriate alert for myself.

Comment on lines 78 to 79
$output->writeln("Last Execution was ".$date->format('Y-m-d H:i:s e').
$elapsedInterval->format("; %h hours, %i minutes, %s seconds ago"));
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should change this to a relative time.

Copy link
Contributor Author

@mortee mortee Feb 16, 2024

Choose a reason for hiding this comment

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

I just added the relative time display, next to the unchanged absolute one. At least for me, it's more relevant how long ago a last update was executed, than at what exact time it was. Can be taken out however, my main goal is the feedback through the exit code, so that scripts can act on it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Problem is that people might already have this scripted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another option could be adding a new command-line switch, which triggers this check, and the exit code. Thus no existing script would possibly break.

Copy link
Contributor Author

@mortee mortee Feb 16, 2024

Choose a reason for hiding this comment

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

I added a --check-elapsed option to the command. Currently, if both are specified, the --reset takes precedence.

I'm thinking either erroring out instead - or, better yet, maybe in that case the job could be automatically reset if it was executed too long ago, along with displaying the message. The docs might warn that this has its own risks, so should be used with caution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, is this good as is, or I should do either of the above suggested changes?

lib/Command/Updater/Job.php Outdated Show resolved Hide resolved
lib/Command/Updater/Job.php Outdated Show resolved Hide resolved
mortee and others added 4 commits February 16, 2024 12:16
Co-authored-by: Sean Molenaar <SMillerDev@users.noreply.github.com>
Signed-off-by: mortee <mortee.github@kavemalna.hu>
Signed-off-by: Nextcloud bot <bot@nextcloud.com>
docs/troubleshooting.md Outdated Show resolved Hide resolved
mortee and others added 2 commits February 16, 2024 14:16
Co-authored-by: Sean Molenaar <SMillerDev@users.noreply.github.com>
Signed-off-by: mortee <mortee.github@kavemalna.hu>
Copy link
Member

@Grotax Grotax left a comment

Choose a reason for hiding this comment

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

Looks good to me now :)

@Grotax Grotax merged commit c926982 into nextcloud:master Feb 21, 2024
19 of 21 checks passed
Grotax added a commit that referenced this pull request Apr 1, 2024
Changed
- make occ news:updater:job exit with code 2 if last update was too long ago (#2590)
- Fix deprecated variable reference in ExportController.php (#2602)
- Add support for Nextcloud 29 (#2611)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
@Grotax Grotax mentioned this pull request Apr 1, 2024
Grotax added a commit that referenced this pull request Apr 1, 2024
Changed
- make occ news:updater:job exit with code 2 if last update was too long ago (#2590)
- Fix deprecated variable reference in ExportController.php (#2602)
- Add support for Nextcloud 29 (#2611)

Signed-off-by: Benjamin Brahmer <info@b-brahmer.de>
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

4 participants