Gather and validate all resource references at one time #5496
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mattwiller
added
the
fhir-datastore
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
ThatOneBro
approved these changes
Nov 5, 2024
| } catch (err) { | ||
| issues.push(createStructureIssue(path, `Invalid reference (${normalizeErrorString(err)})`)); | ||
| const validated = await repo.readReferences(toValidate); | ||
| for (let i = 0; i < validated.length; i ) { |
| const validated = await repo.readReferences(toValidate); | ||
| for (let i = 0; i < validated.length; i ) { | ||
| const reference = validated[i]; | ||
| if (reference instanceof Error) { |
There was a problem hiding this comment.
instanceof in a hot loop is not great, I wish we had a tagged union or something (eg. Option type), but alas.
It's obviously the right choice here just making a note
There was a problem hiding this comment.
@ThatOneBro Is there anything else we should consider here? Would inverting the condition by calling e.g. isResource() instead be better?
There was a problem hiding this comment.
Actually it could improve things slightly, worth a try
There was a problem hiding this comment.
We ran a microbenchmark on both approaches, and discovered that for this use case instanceof Error is about 10x faster than isResource()
|
codyebberson
added a commit
that referenced
this pull request
Nov 7, 2024
This reverts commit bc98343.
codyebberson
added a commit
that referenced
this pull request
Nov 7, 2024
medplumbot
added a commit
that referenced
this pull request
Nov 8, 2024
## What's Changed * Update deb-s3 upload command (#5494) * More build-deb action fixes (#5497) * Added GCP Secret Manager support and configType: gcp (#5462) * Case-insensitive search on ContactPoint (#5455) * Centralize token index type determination logic (#5500) * Gather and validate all resource references at one time (#5496) * Revert "Gather and validate all resource references at one time (#5496)" (#5504) * Improve FHIR Datastore docs (#5484) * Check all references at once (#5508) * Fix QuestionnaireForm dependency on requestSchema (#5509) * fix(agent): harden reconnect logic (#5476) * Tighten up JSON vs FHIR JSON response formatting (#5506) **Full Changelog**: v3.2.19...v3.2.20
Merged
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 8, 2024
## What's Changed * Update deb-s3 upload command (#5494) * More build-deb action fixes (#5497) * Added GCP Secret Manager support and configType: gcp (#5462) * Case-insensitive search on ContactPoint (#5455) * Centralize token index type determination logic (#5500) * Gather and validate all resource references at one time (#5496) * Revert "Gather and validate all resource references at one time (#5496)" (#5504) * Improve FHIR Datastore docs (#5484) * Check all references at once (#5508) * Fix QuestionnaireForm dependency on requestSchema (#5509) * fix(agent): harden reconnect logic (#5476) * Tighten up JSON vs FHIR JSON response formatting (#5506) **Full Changelog**: v3.2.19...v3.2.20
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.