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

Respect maintenance mode when archiving #12339

Merged
merged 2 commits into from Dec 7, 2017
Merged

Respect maintenance mode when archiving #12339

merged 2 commits into from Dec 7, 2017

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Dec 7, 2017

When maintenance mode is enabled the archiver should not start and also not start working on a next website.

On top we could throw an exception in archiveReportsFor() but AFAIK the request will fail anyway since the API will basically return an error. And in archiveReportsFor() should not be like a if (maintenanceEnabled) return because it could result in random bugs as we might set some flags like the site was considered archived. If we want to avoid sending requests eg in archiveReportsFor() we should throw an exception instead like if (maintenanceEnabled) throw new Exception('Maintenance mode is enabled') so the core archiver will stop.

Feel free to tweak messages.

@tsteur tsteur added the Needs Review PRs that need a code review label Dec 7, 2017
@tsteur tsteur added this to the 3.3.0 milestone Dec 7, 2017
@mattab mattab modified the milestones: 3.3.0, 3.2.2 Dec 7, 2017
@mattab
Copy link
Member

mattab commented Dec 7, 2017

LGTM 👍 Just need to wait for tests to be green

@tsteur
Copy link
Member Author

tsteur commented Dec 7, 2017

failing ui tests https://builds-artifacts.piwik.org/piwik/piwik/archivemaintenance/25251/
are not related to this change.

@tsteur tsteur merged commit 6d2a814 into 3.x-dev Dec 7, 2017
@tsteur tsteur deleted the archivemaintenance branch December 7, 2017 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants