-
Notifications
You must be signed in to change notification settings - Fork 1
chore: update seed-script to use new ecr viewer processing endpoint #551
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #551 +/- ##
==========================================
+ Coverage 87.61% 91.47% +3.85%
==========================================
Files 250 115 -135
Lines 13501 4598 -8903
Branches 777 777
==========================================
- Hits 11829 4206 -7623
+ Misses 1664 384 -1280
Partials 8 8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
description="Convert eICR and RR files to FHIR bundles and insert them into the database.", | ||
epilog="For each directory in baseECR the script will look for a `CDA_eICR.xml` and `CDA_RR.xml` file. If they are found, it will convert them into a FHIR bundle (saved as `bundle.json`) and insert that into the database using the Orchestration service.", | ||
) | ||
parser.add_argument( |
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.
is there any way for process-zip
to skip convert? and/or does it always skip convert if there's already a bundle?
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 that ecr-viewer process-zip is only meant to process eicr/rr and does not accept bundle.json anymore
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.
@JNygaard-Skylight will that cause any issues? I think you were the one who added the skipping
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 would be convenient for development and troubleshooting if there were a way to import already converted JSON bundles, especially if we ever get back to having hundreds of eICRs in our development environment. That said, I do not think it is vital.
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.
Looks good to me.
description="Convert eICR and RR files to FHIR bundles and insert them into the database.", | ||
epilog="For each directory in baseECR the script will look for a `CDA_eICR.xml` and `CDA_RR.xml` file. If they are found, it will convert them into a FHIR bundle (saved as `bundle.json`) and insert that into the database using the Orchestration service.", | ||
) | ||
parser.add_argument( |
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 would be convenient for development and troubleshooting if there were a way to import already converted JSON bundles, especially if we ever get back to having hundreds of eICRs in our development environment. That said, I do not think it is vital.
2da552f
to
90e45aa
Compare
90e45aa
to
ebdda59
Compare
|
||
import grequests | ||
|
||
URL = "http://orchestration-service:8080" | ||
UPLOAD_URL = "http://0.0.0.0:3000/ecr-viewer/api/process-zip" |
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.
have you tried using docker.host.internal
?
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've tried host.docker.internal
if that's what you meant.
PULL REQUEST
chore: update seed-script to use new ecr viewer processing endpoint
Summary
Related Issue
Fixes #36
Acceptance Criteria
[ ] All actions, tests, scripts, etc that previously used the /process-zip endpoint on orchestration use the new processing endpoint on the eCR Viewer