Skip to content

Ensure the release succeeds when the primary database is read-only #3562

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

Merged
merged 1 commit into from
Apr 28, 2021

Conversation

jtgeibel
Copy link
Member

@jtgeibel jtgeibel commented Apr 26, 2021

If the primary is in read-only mode then the leader database in
DATABASE_URL may be unavailable. To avoid blocking configuration
changes, the command exits immediately claiming success.

r? @pietroalbini

@pietroalbini
Copy link
Member

Hmm, I would probably change this to straight up skip everything if we're in read-only mode.

If the primary is in read-only mode then the leader database in
`DATABASE_URL` may be unavailable. To avoid blocking configuration
changes, the command exits immediately claiming success.
@jtgeibel jtgeibel force-pushed the fail-migrations-in-read-only branch from c50a4a2 to 4d269b5 Compare April 27, 2021 19:57
@jtgeibel jtgeibel changed the title Ensure the release binary doesn't block when the primary is unavailable Ensure the release succeeds when the primary database is read-only Apr 27, 2021
@jtgeibel
Copy link
Member Author

My interpretation of that page is that the release will fail (so the app won't be restarted), but the configuration changes you made in the UI will not be reverted to the original values.

Yes, that seems like the correct interpretation. I've updated the PR to return success in this situation, and have added some TODO notes regarding future improvements.

@pietroalbini
Copy link
Member

@bors r+

Thanks! Could you open issues to track the TODOs you added?

@bors
Copy link
Contributor

bors commented Apr 28, 2021

📌 Commit 4d269b5 has been approved by pietroalbini

@bors
Copy link
Contributor

bors commented Apr 28, 2021

⌛ Testing commit 4d269b5 with merge e8ce5da...

@bors
Copy link
Contributor

bors commented Apr 28, 2021

☀️ Test successful - checks-actions
Approved by: pietroalbini
Pushing e8ce5da to master...

@bors bors merged commit e8ce5da into rust-lang:master Apr 28, 2021
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.

4 participants