feat(fhircast): add heartbeat event
#3350
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Ignored Deployments
|
Apply Sweep Rules to your PR?
|
ThatOneBro
force-pushed
the
derrick-add-fhircast-heartbeat
branch
from
634917c
to
06c23fb
Compare
ThatOneBro
force-pushed
the
derrick-add-fhircast-heartbeat
branch
from
06c23fb
to
6696581
Compare
ThatOneBro
force-pushed
the
derrick-add-fhircast-heartbeat
branch
from
f0c344f
to
8fae30e
Compare
There was a problem hiding this comment.
Let's discuss next week. My assumption was that the client would send the heartbeat, not the server. If we can avoid it, my preference would be to try to avoid timers in server whenever possible. The whole HeartbeatStore class could be avoided if the client manages the timers.
|
Just implementing it per the EDIT: Also I think relying on the client to send the heartbeat wouldn't fix it for the external clients that we don't manage, since they would still have to implement it themselves. |
|
Potentially we could use one timer and just blast out the heartbeat to all the tracked topics. |
Ahh, good find! I stand corrected. Ok, need to noodle on that. |
ThatOneBro
force-pushed
the
derrick-add-fhircast-heartbeat
branch
from
9a767bd
to
efc0875
Compare
|
@codyebberson I refactored this to use only one timer that gets cleaned up if there are no active topics. Does this make more sense maybe? |
There was a problem hiding this comment.
Nice, thanks @ThatOneBro
See one comment on cleanupPromises array. We can push that to separate commit if you prefer.
Also, as a thought exercise, do you think this could work with the Medplum Agent websocket connection? It will need a similar heartbeat feature. No need to force it if it's too unnatural, but if it fits, that'd be neat.
| const redis = getRedis(); | ||
| const redisSubscriber = redis.duplicate(); | ||
| const deferredCleanupPromise = createDeferredPromise(); | ||
| cleanupPromises.push(deferredCleanupPromise.promise); |
There was a problem hiding this comment.
It looks like we're only adding to cleanupPromises, and never removing? If lots of frequent connect/disconnects, would this array continue to grow? Probably fine in the short term when not many users, but ideally we'd remove promises from the array on websocket close.
There was a problem hiding this comment.
Good point, I think in an earlier iteration I couldn't figure out how to safely remove promises reliably, but I guess when they are resolved they can probably just be removed. I'll fix that
I think that could work. Do you think it would be best just to turn this into a generalized module that gets pulled into both files? Would we want to drive both off of the same timer? |
ThatOneBro
force-pushed
the
derrick-add-fhircast-heartbeat
branch
from
85686f5
to
7854cab
Compare
|
Kudos, SonarCloud Quality Gate passed! |
Fixes #3391 - setup git in preapare-release-yml (#3448) Create example bots that use a shared code library (#3441) Fixes #2789 - support 'delete' subscriptions with bots (#3426) (#3445) Fixes #3391 - github action for prepare-release.sh (#3443) Revert "Fixes #2789 - support 'delete' subscriptions with bots (#3426)" (#3444) Update AWS instructions (#3437) Bump @adobe/css-tools from 4.3.1 to 4.3.2 (#3440) Fix ValidBot warnings (#3438) Fixes #3404 - add Referrer-Policy and Permissions-Policy headers (#3436) Fixes #2789 - support 'delete' subscriptions with bots (#3426) Agent source cleanup (#3431) Stop reviving removed meta properties (#3432) Create docs for building questionnaires (#3393) feat(@medplum/core) Add `AbortSignal` support to client's `createBinary` and `createAttachment` methods (#3420) Fixes #3401 - Handle no AWS credentials in AWS init tool (#3418) Revert "`BinarySource` supporting `ReadableStream` (#3411)" (#3421) Add bulleted list of SuperAdmin priveleges (#3408) Add a note that config must be updated locally when making changes (#3410) Improve linking for using binary data (#3406) Fixes #3413 - set meta.author when updating access policy (#3419) `BinarySource` supporting `ReadableStream` (#3411) Handle non-string code in formatCoding (#3415) Fixes #3414 - use ESM in example apps (#3416) Fixes #3389 - add vercel.json config to examples (#3417) feat(fhircast): add `heartbeat` event (#3350) Add note about graphql respecting access policies (#3403) Fixes #3395 - Build scripts handle no git repo (#3400) Dependency upgrades (#3397) Add callout for batch vs transaction bundles (#3399) Fixes #3395 - Build scripts handle no git repo (#3396) Improve examples in the include search docs (#3359) QuestionnaireBuilder Autosave (#3253) chore(package.json): add `private: true` to root (#3390) Docs: FHIR Profile Creation & Search (#3357)
Fixes #3391 - setup git in preapare-release-yml (#3448) Create example bots that use a shared code library (#3441) Fixes #2789 - support 'delete' subscriptions with bots (#3426) (#3445) Fixes #3391 - github action for prepare-release.sh (#3443) Revert "Fixes #2789 - support 'delete' subscriptions with bots (#3426)" (#3444) Update AWS instructions (#3437) Bump @adobe/css-tools from 4.3.1 to 4.3.2 (#3440) Fix ValidBot warnings (#3438) Fixes #3404 - add Referrer-Policy and Permissions-Policy headers (#3436) Fixes #2789 - support 'delete' subscriptions with bots (#3426) Agent source cleanup (#3431) Stop reviving removed meta properties (#3432) Create docs for building questionnaires (#3393) feat(@medplum/core) Add `AbortSignal` support to client's `createBinary` and `createAttachment` methods (#3420) Fixes #3401 - Handle no AWS credentials in AWS init tool (#3418) Revert "`BinarySource` supporting `ReadableStream` (#3411)" (#3421) Add bulleted list of SuperAdmin priveleges (#3408) Add a note that config must be updated locally when making changes (#3410) Improve linking for using binary data (#3406) Fixes #3413 - set meta.author when updating access policy (#3419) `BinarySource` supporting `ReadableStream` (#3411) Handle non-string code in formatCoding (#3415) Fixes #3414 - use ESM in example apps (#3416) Fixes #3389 - add vercel.json config to examples (#3417) feat(fhircast): add `heartbeat` event (#3350) Add note about graphql respecting access policies (#3403) Fixes #3395 - Build scripts handle no git repo (#3400) Dependency upgrades (#3397) Add callout for batch vs transaction bundles (#3399) Fixes #3395 - Build scripts handle no git repo (#3396) Improve examples in the include search docs (#3359) QuestionnaireBuilder Autosave (#3253) chore(package.json): add `private: true` to root (#3390) Docs: FHIR Profile Creation & Search (#3357)
Closes #3356