-
Notifications
You must be signed in to change notification settings - Fork 1
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
FHIR Gateway Enhancements #92
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #92 +/- ##
============================================
+ Coverage 61.54% 67.61% +6.07%
- Complexity 206 247 +41
============================================
Files 16 16
Lines 1412 1541 +129
Branches 164 184 +20
============================================
+ Hits 869 1042 +173
+ Misses 458 402 -56
- Partials 85 97 +12 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
if (syncLocations.length == 0) | ||
throw new IllegalStateException("No _syncLocation query param specified"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a great idea but should only affect the requests made after the configs are done loading. I am not sure how we can separate that.
We are currently getting this error for good requests that should work without _syncLocation
because they are being requested via that composition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dubdabasoduba how should the Gateway respond for requests that are using REL but do not have _syncLocation specified?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should return an error saying that the sync locations are not set. However, We should ensure the requests made from the composition resources don't throw that error. Those requests are not affected by the sync strategies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think those coming from composition should be ignored on the Allowed Queries or the Ignore Sync filter configuration file.
e3d34f8
to
c82ff20
Compare
56013d5
to
8562e3e
Compare
- Refactor sync strategy fetch
- Fixes REL strategy not returning all records
- Sort sync_locations when generating cache key
9a1acfc
to
9dbbb32
Compare
9dbbb32
to
fd5df53
Compare
- Clean up
43f58bd
to
23ce64a
Compare
IMPORTANT: Where possible all PRs must be linked to a Github issue
In this PR
Engineer Checklist
bug fixes
option(s) on the
README.md
mvn spotless:check
to check my code follows the project'sstyle guide
mvn clean test jacoco:report
to confirm the coverage reportwas generated at
plugins/target/site/jacoco/index.html
mvn clean package
right before creating this pull request.