google / fhir-data-pipes

A collection of tools for extracting FHIR resources and analytics services on top of that data.
https://google.github.io/fhir-data-pipes/
Apache License 2.0
142 stars 82 forks source link

Support for custom FHIR profile #924

Closed chandrashekar-s closed 4 months ago

chandrashekar-s commented 6 months ago

Description of what I changed

Issues Fixed:

560

400

753

558

Pending Changes:

In the long term, we need to provide support for multiple custom profiles. This is being tracked in #980 The issues mentioned in #960 and #961 are partially fixed in the current PR.

E2E test

The application was loaded with custom us-core-profile definitions and the pipeline was run. The tables that were created in the spark thrift server contained the extended definitions defined in the us-core-profile.

image

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 :)

chandrashekar-s commented 6 months ago

@bashir2 The changes have been tested for the Patient resource with us-core-profile. For other resources it was failing because of existing issues in bunsen, will file a bug for the same. Also, for supporting multiple profiles will file another bug.

chandrashekar-s commented 5 months ago

@bashir2 In the latest commit, I have added support for configuring fhir profiles via classpath names. By default, the classpath name containing the us-core fhir profiles have been configured, but user can override them via a different directory as well.

Also, all the us-core fhir profiles definitions have been moved to a common package and is being referenced in all the test cases now.

There are few issues which are open and are being tracked via #960 and #961.

codecov-commenter commented 4 months ago

Codecov Report

Attention: Patch coverage is 53.41246% with 157 lines in your changes are missing coverage. Please review.

Project coverage is 49.61%. Comparing base (ce08411) to head (f39fbc7).

Files Patch % Lines
...java/com/cerner/bunsen/ProfileMappingProvider.java 66.96% 31 Missing and 6 partials :warning:
...ava/com/google/fhir/analytics/PipelineManager.java 18.75% 26 Missing :warning:
.../com/google/fhir/analytics/AvroConversionUtil.java 55.10% 16 Missing and 6 partials :warning:
...c/main/java/com/google/fhir/analytics/FhirEtl.java 0.00% 19 Missing :warning:
...a/com/cerner/bunsen/ProfileMapperFhirContexts.java 67.34% 14 Missing and 2 partials :warning:
.../java/com/google/fhir/analytics/ParquetMerger.java 0.00% 15 Missing :warning:
.../main/java/com/google/fhir/analytics/EtlUtils.java 0.00% 9 Missing :warning:
...erner/bunsen/definitions/StructureDefinitions.java 72.22% 2 Missing and 3 partials :warning:
...erner/bunsen/exception/ProfileMapperException.java 0.00% 2 Missing :warning:
...m/google/fhir/analytics/ProcessGenericRecords.java 0.00% 2 Missing :warning:
... and 3 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #924 +/- ## ============================================ - Coverage 50.10% 49.61% -0.50% - Complexity 605 631 +26 ============================================ Files 86 87 +1 Lines 5073 5196 +123 Branches 632 663 +31 ============================================ + Hits 2542 2578 +36 - Misses 2285 2357 +72 - Partials 246 261 +15 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bashir2 commented 4 months ago

Thanks @chandrashekar-s for the changes; is this ready for another round of review? I mean, are the changes we discussed last week applied?

chandrashekar-s commented 4 months ago

Thanks @chandrashekar-s for the changes; is this ready for another round of review? I mean, are the changes we discussed last week applied?

There are some minor documentation changes pending, will update once done (should be done today).

chandrashekar-s commented 4 months ago

@bashir2 In the latest commit, I have partially addressed the issues listed in #960 and #961. I have also created the issue #980 to create a generic solution for multiple extensions. Updated the description of the PR to reflect the current implementation status.

You can review the last 2 commits now.

chandrashekar-s commented 4 months ago

@bashir2 In the latest commit, I have partially addressed the issues listed in #960 and #961. I have also created the issue #980 to create a generic solution for multiple extensions. Updated the description of the PR to reflect the current implementation status.

You can review the last 2 commits now.

The description also contains a Pending Items section which will be addressed later on, not in this PR.

bashir2 commented 4 months ago

Side note1: The PRs Fixed section of the description should probably be Issues Fixed but even with that I am not sure if that causes merging this PR to close those issues.

Side note2: It is great to see how many issues are being resolved by this PR; great job.