Fixes the changes in handling of text vs zipped resources in recent HAPI versions

muhammad-levi commented 8 months ago

Description of what I changed

Resolves https://github.com/google/fhir-data-pipes/issues/925

E2E test

TESTED:

Please replace this with a description of how you tested your PR beyond the automated e2e/unit tests.

Checklist: I completed these to help reviewers :)

[ ] I have read and will follow the review process.
[ ] I am familiar with Google Style Guides for the language I have coded in.

No? Please take some time and review Java and Python style guides.
[ ] My IDE is configured to follow the Google code styles.

No? Unsure? -> configure your IDE.
[ ] I have added tests to cover my changes. (If you refactored existing code that was well tested you do not have to add tests)
[ ] I ran mvn clean package right before creating this pull request and added all formatting changes to my commit.
[ ] All new and existing tests passed.
[ ] My pull request is based on the latest changes of the master branch.

No? Unsure? -> execute command git pull --rebase upstream master

google-cla[bot] commented 8 months ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

chandrashekar-s commented 8 months ago

Thanks @muhammad-levi for looking into this and raising a PR as well. Looks like as part of the HAPI release 7.0.0, some changes were made to the table HFJ_RES_VER with this PR. This would have caused the code to break.

Also, the pipelines have been tested with HAPI Version 6.1.0 as per this comment.

Will check internally with the team on this and get back to you.

@bashir2 FYI

bashir2 commented 8 months ago

Thanks @muhammad-levi for reporting/fixing this issues and @chandrashekar-s for the investigation; also sorry for the late response. I finally managed to spend some time on this tonight and I think the issue was introduced earlier than changes in HAPI 7.0.0. I can see that the res_text_vc column is being populated in HAPI JPA docker image 6.2.1 which was released at the same time that we anchored the HAPI version to 6.1.0 as @chandrashekar-s pointed out (for e2e test).

In PR #941 I also confirmed that @muhammad-levi's change works with newer versions of HAPI JPA docker image (e.g., 6.10.0) and this PR shows it also works with the older versions, e.g., 6.1.0. However, the problem is that res_text_vc column does not exist in earlier releases of HAPI JPA, e.g., 5.6.0 (I think it was added in this commit) and so this PR will break those versions.

I am going to merge this PR once its e2e test passes. Then in PR #941, I will remove the 6.1.0 constraint. This is not ideal but we can document that for earlier versions of HAPI JPA (2021 and earlier), the JDBC mode does not work and the FHIR API version should be used.

P.S. Once more, this shows the importance of fixing #533.

bashir2 commented 8 months ago

BTW, @muhammad-levi I also updated the title of your PR.

google / fhir-data-pipes