-
Notifications
You must be signed in to change notification settings - Fork 296
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
Database migration exception force closes the app #2170
Comments
@ellykits Can you confirm the engine database version of the app before the SDK update? Ideally, MIGRATION_3_4 should have taken care of this. |
@aditya-07 Apparently, there are no logs other than the one shared. What's the expectation for the migration when the Database version is jumped for instance if the last version is 3 (since the last upgrade) and after the upgrade the new version is 6? Will Room trigger migration one after the other 3 to 4, 4 to 5, and so on? |
Probably the changes that were made in the MIGRATION_3_4 to resolve the issue in |
Yes, the DB will run all the provided migrations sequentially from the last version to the latest version of the database. |
@ellykits can you please dump the db schema for us to take a look? Also about yoru comment:
^ this should have never happened... if it did it's a problem on our part... Can you please pinpoint the commit for us? the only fix i can think of is for you to add a patch (in fhircore) that changes the migration step 4 to 5, in order to "catch up" with the migration steps released in fhir engine... and going forward that shouldn't be a problem. |
more important - @ellykits we really should start thinking about getting fhircore to the released version of engine. @pld @FikriMilano @brynrhodes we should evaluate the level of effort for this one - this could be really beneficial for us in the long run. |
@jingtang10 @aditya-07. After further investigations with @ndegwamartin, we figured the issue to be on our released version of the engine library. In most cases, our releases are ahead of the SDK. Sometimes we merge in unmerged SDK commits as we await the FHIR SDK release. In this particular scenario, we published and used the library with some code changes in the MIGRATION_3_4 migration and missed out on the updates that were later added. |
Thanks @ellykits... I guess in this case my proposed fix still applies. You might just need to change the migration step 4 to 5 to do what you missed out in migration step 3 to 4... does this make sense? This does highlight the need for us to bring fhircore to released versions of engine. The "contract" will be clearer that way and we will be able to better support. Do we think this is achievable imminently or are there still gaps to address? |
@ellykits You should be able to mitigate the issue by adding the missing parts from MIGRATION_3_4 to your MIGRATION_4_5 and updating the app. |
@
Yes, that does. This will resolve our current issue. |
That's definitely what we want. The ongoing challenge is when we have a hard deadline (driven by partner timelines related to the movement of people) and we need to use code that isn't yet in a versioned release. This is trending in the correct direction, towards on using released versions as Android FHIR SDK and OpenSRP 2 become more mature |
I have managed to successfully apply the latest migrations. The issue can be closed at your convenience @jingtang10, if Peter's comment above is satisfactory. |
Thanks Peter. Can we draw up a list of issues blocking this shift? If you give me some pointers I can start a tracker. |
@jingtang10 our custom artifact releases contain the following unmerged PRs cc @pld @ellykits |
Describe the bug
Migration exception causing app crash
Component
Core library
To Reproduce
Steps to reproduce the behavior:
Upgrade to the latest version of SDK. Run an existing application. Experience application crash.
Expected behavior
Database Migration should run successfully without causing any app crash.
Additional context
Logs
I cleaned up the TableInfo logs, did a diff-check, and found an issue in the
indices
section.Expected
Found
Would you like to work on the issue?
Sure
The text was updated successfully, but these errors were encountered: