Support for VSAC FHIR API #28
Conversation
Add support for FHIR with little change to overall structure and organization of the code.
- Refactor code to reduce duplication and minimize footprint of different APIs (FHIR vs SVS). - Use async / await where it makes sense - Refactor and update tests
There was a problem hiding this comment.
I think the refactor makes a lot of sense and cleans things up nicely! I ran the tests and the manual-test.js script and looked through the diffs, and things all made sense to me. I just left one really small comment, but I suspect there's a reason so I don't think it should hold up my approval.
| // TODO: Throw error instead | ||
| throw 'Failed to download value sets since UMLS_API_KEY is not set.'; |
There was a problem hiding this comment.
Is there a reason you can't throw the error now?
There was a problem hiding this comment.
This preserves the same user-facing behavior as before. My only concern was that if someone is using this library and expecting it to reject with a string -- and we change it to reject with an error, then their use might break. So changing it to an Error would be a breaking change. But... we reject with errors in a lot of other places, so in the future, it would be more consistent to do that. Basically, we'll do it whenever we next make a breaking change.
This PR adds support for the VSAC API by adding a
useFHIRboolean parameter to theCodeServiceconstructor. The SVS API is still used by default, as it seems to be faster than the FHIR API.I've also taken the opportunity to reorganize the code a bit so that the SVS-only constructs and FHIR-only constructs are in their own files while the common constructs are in the main class file. This includes reorganization of the tests to more clearly separate out the concerns of the different APIs vs the common
CodeServicefunctions. In addition, I've converted much of the promise-handling logic to useasync/await.You can run a quick manual test by running
npm installand thennode manual-test.js. There are slight differences in output (e.g., no code system versions in the FHIR output), but this is expected as there are differences in the data each API returns (e.g., for this value set, the FHIR API doesn't return a code system version).