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

Allow maintenance mode to be set in config.js #7124

Merged
merged 1 commit into from
Jul 25, 2016

Conversation

ErisDS
Copy link
Member

@ErisDS ErisDS commented Jul 23, 2016

This change makes it possible to test maintenance mode by setting the following in config.js:

maintenance: {
  enabled: true
}

However, I have a hunch that setting maintenance mode regardless of whether there is going to be a migration can cause the first request on boot to be a Maintenance Error.

I think we need to change this to only set maintenance mode if there is work to be done.

refs #6976, #7019, #7125

  • Ensure maintenance mode flag is set back to what is in config.js rather than defaulted to false on boot
  • Remove stack trace from 503 errors
  • Add error message to 503 error
  • Ensure error page is rendered for Ghost-Admin on reload with 503

@ErisDS ErisDS mentioned this pull request Jul 23, 2016
5 tasks
@ErisDS ErisDS force-pushed the no-maintenance-override branch from c338799 to 6fb514b Compare July 23, 2016 10:50
@ErisDS ErisDS changed the title Allow maintenance mode to be set in config.js [WIP] Allow maintenance mode to be set in config.js Jul 23, 2016
@kirrg001
Copy link
Contributor

all changes look good to me 🎉
one little thing: if the 503 page is rendered, there is an anchor go to the front page, which is in this case maybe a bit confusing.

refs TryGhost#6976, TryGhost#7019, TryGhost#7125

- Ensure maintenance mode flag is set back to what is in config.js rather than defaulted to false on boot
- Remove stack trace from 503 errors
- Add error message to 503 error
- Ensure error page is rendered for Ghost-Admin on reload with 503
@ErisDS ErisDS force-pushed the no-maintenance-override branch from 53dbd86 to 54388e1 Compare July 25, 2016 16:14
@ErisDS
Copy link
Member Author

ErisDS commented Jul 25, 2016

Totally agree about the go to front page thing being confusing, I think that's actually true for a lot of cases and we're going to need to seriously improve how errors are rendered across the admin vs blog very soon.

For this release, I can't think of a quick fix that doesn't involve adding extra booleans or a helper to errors to be able to detect the error type / location, which then impacts on the theme API. Seems like overkill for this release but definitely want to revisit error rendering sooooooon.

@ErisDS ErisDS changed the title [WIP] Allow maintenance mode to be set in config.js Allow maintenance mode to be set in config.js Jul 25, 2016
@ErisDS
Copy link
Member Author

ErisDS commented Jul 25, 2016

P.S. I updated this to be one commit and have a full commit message

@kirrg001 kirrg001 merged commit d08926c into TryGhost:master Jul 25, 2016
chris-brown pushed a commit to chris-brown/Ghost that referenced this pull request Aug 14, 2016
refs TryGhost#6976, TryGhost#7019, TryGhost#7125

- Ensure maintenance mode flag is set back to what is in config.js rather than defaulted to false on boot
- Remove stack trace from 503 errors
- Add error message to 503 error
- Ensure error page is rendered for Ghost-Admin on reload with 503
geekhuyang pushed a commit to geekhuyang/Ghost that referenced this pull request Nov 20, 2016
refs TryGhost#6976, TryGhost#7019, TryGhost#7125

- Ensure maintenance mode flag is set back to what is in config.js rather than defaulted to false on boot
- Remove stack trace from 503 errors
- Add error message to 503 error
- Ensure error page is rendered for Ghost-Admin on reload with 503
@ErisDS ErisDS deleted the no-maintenance-override branch March 1, 2017 12:47
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.

2 participants