1.11.4 upgrade fails due to person change to datetime

Development
21

Then, a few things:

  1. I guess it’s okay to proceed with the Reference Application 2.3 release based on Platform 1.11.4 (as long as @sunbiz confirms he has auto-update=true, and he didn’t hit some other bug)
  2. In that case we need to flag in the documentation of Reference Application 2.3 and Platform 1.11.4 that you cannot upgrade with auto-update=true
  3. We need to prioritize TRUNK-4726 ! Rafal curated this ticket and marked it as Priority = Must on June 3 (and also “intro” a bit later), however in the 3+ months since then it hasn’t been touched. (Unfortunately Platform 1.11.4 was released during the 9-day span where this ticket incorrectly lost its 1.11.4 fixVersion.)
  • I think it’s misleading to have a ticket in openmrs-core that is marked with priority=Must that goes 3+ months without being worked on. “Must” is weaker than “Blocker” but to me it conveys more than “whenever someone picks this up”. @dkayiwa, @wyclif, @raff, and others, could you all review the process for curating and prioritizing tickets, and see whether/how it needs to be modified to ensure that marking a ticket as higher priority actually has some impact?
22

I took sometime to reproduce and deep dive into the cause of this, it turns out that Daniel is correct that you should be able to upgrade to 1.11.4 if you have allow_auto_update set to false which is the default but it will fail if it is set to true. By the way, this feature worked in the past i.e run updates when allow_auto_update set to true but has been broken since 1.11.0, when it is set to true updates are automatically run by Context.startup(). The bug is caused by this commit/fix for TRUNK-344, which as seen here is fetching a GP through the API before OpenMRS is started (VERY UNACCEPTABLE) :blush: , of course the authorization advice around this service method tries to do its work but when it calls Context.getAuthenticatedUser() it fails as this method tries to load the daemon user, because the updates haven’t been run yet, you get the error @sunbiz reported.

The fix is it to move the line that fetches the GP through the API to happen after the call to Context.startup() since the updates at that point have already been run even when allot_auto_update is set to true OR use SQL to fetch it if it needs to be used before the service context is up. This also makes me think that it is ironic that we provide a mechanism to notify modules that openmrs has been started so they can be able to set certain configurations but in core we don’t have a well designed way to guide DEVs to know the correct place to add code like this which initializes certain things on startup to avoid similar bugs in the future.

With that said, this shouldn’t block the 2.3 release given that the bug has actually been around since 1.11.0

1 Like
23

Thanks, @wyclif and @dkayiwa, for taking the time to deep-dive on this!

Please also make sure that any work required to fix things up is properly ticketed and prioritized.

1 Like
24

I’ve updated the ticket description to say what needs to be done

25

Looks like there was this duplicate ticket reported even earlier:

26

Closed TRUNK-4773 since it duplicates TRUNK-4726