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

Possible upgrade message mitigation #19982

Closed
wants to merge 1 commit into from

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Not sure if this is a good idea or not but I'm trying to think about
https://lab.civicrm.org/dev/core/-/issues/2474 and what we can possiby do
to help people with weird DBs

I don't think this error has to be fatal - the situation is likely not worse than before if it fails

@totten @seamuslee001 @colemanw

Before

What is the old user-interface or technical-contract (as appropriate)?
For optimal clarity, include a concrete example such as a screenshot, GIF (LICEcap, SilentCast), or code-snippet.

After

What changed? What is new old user-interface or technical-contract?
For optimal clarity, include a concrete example such as a screenshot, GIF (LICEcap, SilentCast), or code-snippet.

Technical Details

If the PR involves technical details/changes/considerations which would not be manifest to a casual developer skimming the above sections, please describe the details here.

Comments

Anything else you would like the reviewer to note

Not sure if this is a good idea or not but I'm trying to think about
https://lab.civicrm.org/dev/core/-/issues/2474 and what we can possiby do
to help people with weird DBs
@civibot
Copy link

civibot bot commented Apr 7, 2021

(Standard links)

@civibot civibot bot added the master label Apr 7, 2021
@demeritcowboy
Copy link
Contributor

Thoughts:

  1. This may be fixed by Fix loop #19858 (comment), which wouldn't have been in the version in the lab ticket.
  2. postupgradeMessage is actually called at the START of the upgrade process and can't report on anything that happens during the upgrade steps. I've come up against this a couple times.

@eileenmcnaughton
Copy link
Contributor Author

Oh yeah - I had flushed that creepiness from my mind.

I guess the try catch might make sense but there would be no feedback

@eileenmcnaughton
Copy link
Contributor Author

Closing for now - can still be a discussion piece closed but it is not reviewable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants