Add RR extraction logic, phdi dependency to FHIR converter #814
Conversation
… into emma/737-really
… into emma/737-really
… emma/737-part-2
Codecov Report
@@ Coverage Diff @@
## main #814 +/- ##
=======================================
Coverage 96.42% 96.42%
=======================================
Files 45 45
Lines 2626 2626
=======================================
Hits 2532 2532
Misses 94 94
Flags with carried forward coverage won't be shown. Click here to find out more. |
| @@ -98,14 +98,16 @@ def convert_to_fhir( | |||
| # Process the response from FHIR Converter. | |||
| if converter_response.returncode == 0: | |||
| result = json.load(open(output_data_file_path)) | |||
| old_id = None | |||
There was a problem hiding this comment.
Required for the Python 3.10 update 🤷
containers/fhir-converter/tests/integration/test_FHIR-Converter.py
Outdated
Show resolved
Hide resolved
emmastephenson
requested review from
nickclyde,
BradySkylight and
DanPaseltiner
There was a problem hiding this comment.
Thanks @emmastephenson this looks good to me. Left one minor suggestion and a question below, but reason for either to be blocking.
| rr_data: str = Field( | ||
| description="If an eICR message, the accompanying Reportability Response data.", | ||
| example=["sample"], | ||
| default=None, | ||
| ) |
There was a problem hiding this comment.
I think we should let this be optional, so that the docs don't show it as *required?
There was a problem hiding this comment.
agreed and good catch.
There was a problem hiding this comment.
Good catch, thank you! Updated.
| if input.rr_data is not None: | ||
| if input.root_template != "EICR" or input.input_type != "ecr": | ||
| response.status_code = status.HTTP_422_UNPROCESSABLE_ENTITY | ||
| result = { | ||
| "message": "Reportability Response (RR) data is only accepted " | ||
| "for eCR conversion requests." | ||
| } |
| # Install python via pyenv | ||
| RUN apt-get update && apt-get install -y curl git build-essential libreadline-gplv2-dev libncursesw5-dev libssl-dev libsqlite3-dev tk-dev libgdbm-dev libc6-dev libbz2-dev libffi-dev zlib1g-dev | ||
| RUN curl https://pyenv.run | bash | ||
| RUN export PYENV_ROOT="$HOME/.pyenv" | ||
| RUN export PATH="$PYENV_ROOT/bin:$PATH" | ||
| RUN eval "$(pyenv init -)" | ||
| RUN /root/.pyenv/bin/pyenv install 3.10.13 | ||
| RUN /root/.pyenv/bin/pyenv global 3.10.13 | ||
|
|
||
| RUN /root/.pyenv/shims/pip install --no-cache --upgrade pip setuptools |
There was a problem hiding this comment.
Why do we need to use pyenv here? It's fine, I'm just curious.
There was a problem hiding this comment.
More or less because there weren't other options that allowed us to install a specific Python version on top of the dotnet image. We originally tried building Python 3.7 from source but that didn't work, so pyenv ended up being the most straightforward way to install everything.
PULL REQUEST
Summary
Adds RR extraction logic to the FHIR converter.
This also adds a dependency on
phdi, which in turn required some substantial changes to the Dockerfile to ensure we're installing Python 3.10 (the old way installed 3.7, which wasn't compatible with some of the phdi dependencies.)Related Issue
Fixes #737
Additional Information
One more planned PR in this effort - adding RR extraction to the validation service.
There's also a bunch of integration test work that I split into a separate PR - #826.