Stream: python
Topic: multi-version fhirclient
Andrew Wason (Oct 20 2019 at 13:38):
Moving this discussion https://groups.google.com/d/msg/smart-on-fhir/pqwmqTMYIzk/fF9y7V8DEAAJ here.
I have a PR to make fhirclient support multiple FHIR versions here https://github.com/smart-on-fhir/client-py/pull/81
What is the best way to move forward with this? I could do a new PR that simply repackages the R4 models in a backward compatible way, then add support for DSTU2 and STU3 in a subsequent PR.
Also, currently on fhirclient master most of the python 2.7 unit tests fail, I had fixed this in my PR - but should we just drop 2.7? Or should I just ignore the failures for now to minimize the size of the PR (fixing them requires regenerating all the unit test files)?
Ward Weistra (Oct 21 2019 at 08:20):
Thanks so much for your contribution, Andrew!
I think @Pascal Pfiffner indeed suggested to break the PR up in multiple parts, to make it easier to digest. Your proposed break down seems to make sense, but I couldn't say for sure. Probably @Harold Solbrig and Pascal are most qualified to judge? Or maybe @Adam Culbertson @Carl Anderson?
Pascal Pfiffner (Oct 21 2019 at 15:53):
IMO the best way is to first move the existing models into a R4 subfolder and in follow-up PRs add the older versions back in. All other stuff like config changes should be done in separate PRs as well. But aware, @Harold Solbrig 's team has been working on changes over the summer that should be ready soon, we probably want to wait for these before anything major lands. I think we decided to drop Python 2.x.
Andrew Wason (Oct 21 2019 at 15:59):
@Pascal Pfiffner OK, I repackaged R4 in this PR https://github.com/smart-on-fhir/client-py/pull/82
Pascal Pfiffner (Oct 21 2019 at 16:07):
This will still have to wait for Harold's changes, and I'd also pull the CI integration out into a separate PR. FWIW I'm not the maintainer anymore.
Andrew Wason (Oct 21 2019 at 16:41):
What branch is he working on so I can see if there will be any tricky conflicts? We need to use multiple versions in our app so hopefully this won't take too long. If the conflicts aren't that bad, we could merge either PR first and rebase/merge the other branch from master.
Andrew Wason (Oct 23 2019 at 21:30):
So since we need to support DSTU2, we switched back to v1.0.3 of fhirclient. We use an app_secret, but it looks like v1.0.3 has no support for that. So must we wait for my future series of PRs to be merged before we can use this?
Andrew Wason (Oct 29 2019 at 18:22):
@Harold Solbrig any timeframe for when you are going to land your changes?
Adam Culbertson (Nov 02 2019 at 19:06):
Thanks so much for your contribution, Andrew!
I think Pascal Pfiffner indeed suggested to break the PR up in multiple parts, to make it easier to digest. Your proposed break down seems to make sense, but I couldn't say for sure. Probably Harold Solbrig and Pascal are most qualified to judge? Or maybe Adam Culbertson Carl Anderson?
Sure I can connect with Harold and see how we can keep this moving forward.
Andrew Wason (Nov 26 2019 at 20:26):
Any response from Harold?
Grahame Grieve (Nov 27 2019 at 04:51):
@Harold Solbrig are you around?
Brian Kolowitz (Nov 27 2019 at 14:36):
i've had a pull request sitting out there since may that i believe fixes part of this issue https://github.com/smart-on-fhir/client-py/pull/66 pascal commented and said it looks good, just need someone to merge it in
Ward Weistra (Nov 27 2019 at 15:27):
I've seen emails that @Harold Solbrig and likely @Duncan Weatherston had a community call on this, and notes should follow... I'll ping him by email again.
Harold Solbrig (Nov 27 2019 at 16:49):
I am now -- I kind of lost track of zulip so I hadn't been aware that this conversation was going on
Harold Solbrig (Nov 27 2019 at 16:49):
Need to catch up and will respond shortly
Harold Solbrig (Nov 27 2019 at 16:55):
Most immediate need - the pull request. Accepted and merged
Harold Solbrig (Nov 27 2019 at 16:57):
Next thing -- I've proposed separating the client.py code (server.py / auth.py) from the python rendering of the fhir models. What is the best way to proceed to discuss, etc. this item?
Andrew Wason (Nov 27 2019 at 17:04):
@Harold Solbrig I did that in my original PR, I package each fhir version models in separate python packages and then allow the user to select which version they want to use. See my branch here https://github.com/smart-on-fhir/client-py/compare/master...rectalogic:versioning
This was my original PR, but I was asked to introduce it gradually. So currently I have a PR that just repackages the R4 models here https://github.com/smart-on-fhir/client-py/pull/82 - but I was told this cannot be merged until some changes you are working on have been merged. So I have been waiting a month for your changes to land so that this PR can be reviewed. Once that is done I will submit another PR for the rest of the changes that support DSTU2 and STU3 simultaneously with R4.
Harold Solbrig (Nov 27 2019 at 17:06):
Apologies for the wait. Should we have a short voice chat to sync up? I could be available on Friday...
Andrew Wason (Nov 27 2019 at 17:15):
Next week would work better for me, pretty much any day next week. @Harold Solbrig let me know what day/time works for you
Andrew Wason (Nov 27 2019 at 18:41):
Hmm, fhirclient test_models.sh relies on the downloaded json spec files
https://github.com/smart-on-fhir/client-py/blob/master/test_models.sh
But these recently changed - http://hl7.org/fhir/R4/examples-json.zip used to be 4.0.0 but is now 4.0.1. So fhir-parser/generate.py --load-only downloads the 4.0.1 specs.
Is there somewhere to download the 4.0.0 spec files since that is what fhirclient models were generated using - all the unit tests fail when using the 4.0.1 spec files.
Ward Weistra (Nov 28 2019 at 12:46):
It could be part of the Zip download here: http://hl7.org/fhir/directory.html
Grahame Grieve (Nov 28 2019 at 15:52):
shouldn't be any different for the examples?
Pascal Pfiffner (Nov 28 2019 at 23:00):
Only when generating new models do the unit tests need to run so one can validate the generation. Maybe they should be spun out of the client?
Andrew Wason (Dec 02 2019 at 14:47):
I'll change my PR to make running those unit tests a separate task that can be run only when validating new models.
Andrew Wason (Dec 02 2019 at 15:28):
So what's the plan now? Can someone review my initial PR https://github.com/smart-on-fhir/client-py/pull/82 and then we can move on to packaging the DSTU2 and STU3 models and support multiple versions? Or do we still need to wait for @Harold Solbrig to merge his changes first?
Andrew Wason (Dec 12 2019 at 15:49):
I'm going to close my PR. We haven't been able to get any traction with this and so we ended up just writing or own SMART on FHIR implementation that works with any version and meets our needs.
Ilya Beda (Jan 07 2020 at 10:03):
Hi, @Andrew Wason !
Are you going to opensource your solution?
Did you check https://github.com/beda-software/fhir-py?
We support multiple FHIR versions out of the box.
Andrew Wason (Jan 07 2020 at 15:10):
Are you going to opensource your solution?
Did you check https://github.com/beda-software/fhir-py?
I did check out fhir-py, it didn't seem to support SMART on FHIR which we need. We need to use SMART to authenticate the EMR user with our application (SSO) and our solution is kind of tightly coupled with our existing SSO implementations so I don't think I'll try to extract it to opensource it. Now that we have the SMART piece implemented I think we may end up using fhir-py in the future. For now we only needed one FHIR api so I just did a simple implementation using requests.
Ilya Beda (Jan 08 2020 at 10:04):
Thank you @Andrew Wason for the detailed answer,
we are going to add SMART on FHIR support soon.
Why do you decide to use requests over asyncio for the HTTP layer?
We are still debating regarding supporting requests in parallel with our primary technology - asincio.
Maybe your use cases can show where requests may be preferable.
Andrew Wason (Jan 08 2020 at 15:23):
I don't see an easy way to incrementally adopt asyncio. This is in a pyramid/uwsgi-based webapp and using mongodb database with pymongo. So we would have to rewrite all the database code using an async driver like motor, move to some async asgi server instead of uwsgi and so on. It seems like this would mean replacing nearly every part of our technology stack and rewriting all the code based on it.
Last updated: Apr 12 2022 at 19:14 UTC
