Stream: subscriptions
Topic: Notes on Subscriptions
nicola (RIO/SS) (Jan 14 2020 at 10:16):
Subscription.eventCount a does not look like a good element for definition resource - it's part of something like SubscriptionStatus. Logical check - when I PUT Subscription with eventCount=0 - what does it mean? The same about Subscription.error. Do you consider introduce SubscriptionStatus resource or at least move all these status attributes in complex element like Subscription.status = {error, count}
?
nicola (RIO/SS) (Jan 14 2020 at 10:17):
We can use same pattern as k8s - https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#pod-and-container-status
nicola (RIO/SS) (Jan 14 2020 at 10:18):
Subscription.channel.type is CodeableConcept! Do you expect such a complicated classification? Why code is not enough (or Coding)?
nicola (RIO/SS) (Jan 14 2020 at 10:23):
For elements like contact, reason, name etc - can we keep Sub elements at a minimum required? Such meta-info can be added by extensions - if extensions will become popular - after couple of years - move it to the core.
nicola (RIO/SS) (Jan 14 2020 at 10:25):
Element heartbeatPeriod is very implementation-specific (80/20?). I would suggest publishing official extensions for such cases
nicola (RIO/SS) (Jan 14 2020 at 10:27):
Structure of filterBy looks kinda naive - I can imagine a lot of different filter engines - can we make it extensible by design?
nicola (RIO/SS) (Jan 14 2020 at 10:29):
Something like: filter: { type: code, expression: any, ....};
nicola (RIO/SS) (Jan 14 2020 at 10:43):
Instead of playing this "bundle reuse game" why not use specialized envelope:
type: handshake | notification | heartbit event: created timestamp: ...... resource: resourceType: Patient id: pt-1 .....
nicola (RIO/SS) (Jan 14 2020 at 13:37):
For web-hooks as we know timeout
parameter is quite important
nicola (RIO/SS) (Jan 14 2020 at 13:41):
It would be nice to be able to "inject" constant context data into payload for multi-service subscribers.
Josh Mandel (Jan 14 2020 at 14:02):
Subscription.eventCount a does not look like a good element for definition resource - it's part of something like SubscriptionStatus. Logical check - when I PUT Subscription with eventCount=0 - what does it mean? The same about Subscription.error. Do you consider introduce SubscriptionStatus resource or at least move all these status attributes in complex element like
Subscription.status = {error, count}
?
Indeed, now that we're adding SubscriptionStatus, these will migrate to SubscriptionStaus
Josh Mandel (Jan 14 2020 at 14:04):
Subscription.channel.type is CodeableConcept! Do you expect such a complicated classification? Why code is not enough (or Coding)?
Agreed -- needs to be Coding to allow for extensibility, but not CodeableConcept.
Josh Mandel (Jan 14 2020 at 14:05):
For elements like contact, reason, name etc - can we keep Sub elements at a minimum required? Such meta-info can be added by extensions - if extensions will become popular - after couple of years - move it to the core.
Gino and I would have left all these out. @Lloyd McKenzie has kind of insisted that these be included "because the definition pattern requires them" (not a verbatim quote). I'd happily trim these...
Josh Mandel (Jan 14 2020 at 14:07):
Heartbeat is an interesting one: the details change but the concept is relevant for many channel types including our most important ones today (rest hook, web socket), so I'm not sure I like the idea of taking it away from the core design.
Josh Mandel (Jan 14 2020 at 14:12):
Regarding, FilterBy: the super flexible "code, expression" -- is your comment here just about Subscription.filterBy, or also about SubscriptionTopic.canFilterBy? (There, we avoided the totally generic approach, because it didn't let us express the things we actually wanted, like
queryCriteria": { "previous": "status:not=in-progress", "current": "status=in-progress", "requireBoth": true },
Josh Mandel (Jan 14 2020 at 14:14):
As for a specialized envelope type, I'm sort of ambivalent. It's kind of charming that FHIR only has one resource that directly embeds others ("contained" shenanigans aside). What would the main gain be, over defining a Bundle.type of "subscription"?
Josh Mandel (Jan 14 2020 at 14:14):
BTW Nicola, thanks much for the comments and close read!
nicola (RIO/SS) (Jan 14 2020 at 14:15):
Bundle.type may work with subs-handshake | subs-heartbeat | subs-notification
nicola (RIO/SS) (Jan 14 2020 at 14:17):
I would like to see event name like 'created', 'updated', 'deleted' instead of parsing HTTP string
nicola (RIO/SS) (Jan 14 2020 at 14:17):
But that's probably can be done as an extension of Bundle.entry
nicola (RIO/SS) (Jan 14 2020 at 14:18):
Do we specify that this bundle should always contain one resource? Or it can be batch notification?
nicola (RIO/SS) (Jan 14 2020 at 14:20):
@Josh Mandel what do you think about channel.timeout
- as we see from our trial it's quite important for server and client. Otherwise, it's hard to find the implicit configuration without confusing one of them
nicola (RIO/SS) (Jan 14 2020 at 14:23):
As well unique id of the notification may be helpful for tracking duplicates ("exactly one" semantic is hard to make work)
Lloyd McKenzie (Jan 14 2020 at 15:41):
I'm pretty sure Lloyd didn't insist on adding those. Following the Definition pattern is expected for resources that are defining what kinds of activities are possible to occur. Of the various resources in the subscription space that might meet that intention, only 'Topic' really meets that definition. Subscription is an event, not a definition. (It would be nice if you considered alignment with the Event pattern there - though as usual, alignment with a pattern is "consider these elements, either as core or as extensions and consider aligning with these names". Patterns never force a resource to include anything.
Gino Canessa (Jan 14 2020 at 17:09):
Ok, think I'm catching up.. some of my comments will echo Josh's, but I need to consolidate for sanity. First, @nicola (RIO/SS), thanks for the feedback! I have a few questions on items below, if you wouldn't mind clarifying/giving context. If there's anything I missed or have wrong, please let me know as well.
-
re:
Subscription.eventCount
: Yes,eventCount
will to move into theSubscriptionStatus
resource (drafting in progress). As a note, both theeventCount
andstatus
fields had (or would have) guidance regarding what values acceptable to PUT/POST (e.g., initial requests, resetting from errors, etc.). Beyond that, it would be up to the server implementer to decide what to do (e.g., reject, accept modified, etc.). -
re: data type of
Subscription.channel.type
:Code
is an issue because of extensibility. I do like moving it toCoding
though. I will ask to see if anyone has a reason for not changing it. -
re: general fields: Echoing Josh, many of these fields have been requested for resources in general (from knowledge gained in other areas). They are optional and not too many, so what is the driving concern to oppose them?
-
re:
Subscription.channel.heartbeatPeriod
: Josh discussed, but to summarize: we've felt this is core to the spec and useful across all channel types. -
re:
Subscription.filterBy
: These are paired withSubscriptionTopic.canFilterBy
elements. We've (so far) found that using Search parameters and modifiers allow for powerful filtering, while not over-committing servers. What use case do you have that doesn't fit this? -
re: "bundle reuse game": Current feedback is to define a new bundle type and include the new (coming soon)
SubscriptionStatus
resource as the firstentry
in the bundle (similar in concept to a message header). This allows for those types to be first-class members and should clean all that up. Does that work, or do you still have concerns? -
re:
timeout
parameter: Is that important for a client to be able to set? I've always felt it was a server concern (e.g., what I will allow for a timeout on http calls). I'm not opposed to proposing the add, but would like to understand more context around why you feel so strongly, if possible. Ahh, looks like this is from another area that it became important (leaving original notes for myself). I'll put it on the list to add. -
re: event types: you don't need an extension (why we used
history
and will also be allowed in the new bundle type), you can just useBundle.request.method
and include theHTTPVerb
Edit: fixed formatting, bolded questions for Nicola
Gino Canessa (Jan 14 2020 at 17:25):
Missed one, the server is allowed to batch notifications together. That is part of justification for bundleEventCount, so that empty notifications can represent multiple events.
Eric Haas (Jan 14 2020 at 18:32):
re patterns and Lloyd insisting... I insisted on the pattern elements in Topic but for Subscriptions I don't see any pattern. in fact seems more like a request than event. Those elements were inherited from the R4 subscriptions and we added name.
Eric Haas (Jan 14 2020 at 18:37):
I think what @nicola is asking for in the Bundle game comment is more that a subscription type, but a whole heirarchy of types .... e.g subscription-event, subs-handshake | subs-heartbeat | subs-notification
Eric Haas (Jan 14 2020 at 18:39):
and I am not understand how bundleEnevtCount, = batch notification if the notification is not empty.
Eric Haas (Jan 14 2020 at 18:42):
and @nicola asked for a "event trigger code"
in SubscriptionHeader
I would like to see event name like 'created', 'updated', 'deleted' instead of parsing HTTP string
is that something that is being considered?
Gino Canessa (Jan 14 2020 at 18:44):
and I am not understand how bundleEventCount, = batch notification if the notification is not empty.
The issue we discovered was if you have Subscription.channel.payload.content
set to empty
, there are no contents to index from. So, a bundle representing a single event and one batched to include 10 would look identical. Adding the field bundleEventCount
makes it so the server can communicate how many events are included in the notification, which clears that up.
Edit: In the cases of id-only
or full-resource
payloads, batching is detectable without the bundleEventCount
(since each entry would represent one notification), but the group felt the count was still useful in those cases.
Edit #2: I'll also note that there has been discussion about how to describe server batching and/or responsibilities (e.g., if a server wants to batch per second, that's within general expectations - if a server wants to batch per day, that wouldn't be). I don't think anything more concrete than "yeah, that should be addressed" has come out of it yet.
Gino Canessa (Jan 14 2020 at 18:51):
and @nicola asked for a "event trigger code"
in SubscriptionHeaderI would like to see event name like 'created', 'updated', 'deleted' instead of parsing HTTP string
is that something that is being considered?
Right now, we have been assuming Bundle.request.method
and Bundle.response.status
can be used to clarify anything ambiguous from the topic.
Re-reading, I guess the ask is for another field for specific FHIR interactions (e.g., update, patch, create, etc.)? I'm not sure how I feel about that given how much discussion we're having about reducing the number of fields.
Gino Canessa (Jan 14 2020 at 19:07):
and @nicola asked for a "event trigger code"
in SubscriptionHeaderI would like to see event name like 'created', 'updated', 'deleted' instead of parsing HTTP string
is that something that is being considered?
Splitting this up for my sanity (re: response codes above), there is a separate discussion (short, in the State Changes Blog thread) about adding to triggers around this (for SubscriptionTopic definitions).
Probably buried enough that I'll start a new thread about just that.
Grahame Grieve (Jan 14 2020 at 19:24):
Can we discuss extensibility on the channel type element? The current channel list is rest-hook | websocket | email | message
. What additional channels would be considered as grounds for extensibility, and able to be added using the existing elements?
Grahame Grieve (Jan 14 2020 at 19:25):
also, Subscription.channel.payload seems like a pointless element. we should collapse it's children into the parent
Paul Church (Jan 14 2020 at 19:33):
re: extensible channel types - we'll support Google Cloud Pub/Sub, the channel.endpoint is the pubsub topic. It's not a full URL, just a GCP resource path like "projects/X/topics/Y". There might be a similar pattern for Kafka and other message queues.
Grahame Grieve (Jan 14 2020 at 19:34):
you'll have to define some scheme prefix for the GC Pub/sub
Paul Church (Jan 14 2020 at 19:36):
or put it in an extension
Grahame Grieve (Jan 14 2020 at 19:38):
... maybe. but it's surprising that it doesn't already have a scheme to me
Gino Canessa (Jan 14 2020 at 19:38):
Can we discuss extensibility on the channel type element? The current channel list is
rest-hook | websocket | email | message
. What additional channels would be considered as grounds for extensibility, and able to be added using the existing elements?
Off the bat, I can see something like a message-queue channel (for reliable delivery on servers that want to support it). Similarly, things like Azure Event Hub (or their AWS or GCP equivalents) would be useful to people building on those platforms.
In most of the cases people have discussed with me, the fields we have are good for the base but they will probably need an extension or two (e.g., a public key, etc.).
Gino Canessa (Jan 14 2020 at 19:39):
also, Subscription.channel.payload seems like a pointless element. we should collapse it's children into the parent
Works for me.
Gino Canessa (Jan 14 2020 at 19:43):
(re: type extensibility) Or, are you suggesting changingSubscription.channel.type
to Coding
?
Paul Church (Jan 14 2020 at 19:43):
Hmm, I guess it's just https://pubsub.googleapis.com/v1/projects/X/topics/Y.
Grahame Grieve (Jan 14 2020 at 19:47):
there's 4 ways to handle channel:
- not to be extensible (my preference but I can see that won't win)
- to make it a
code
and use some external system (a la mime type) - make it a coding and define our own codes
- make it a CodeableConcept (worst option)
Grahame Grieve (Jan 14 2020 at 19:47):
is there a vocabulary from some other external rfc on this matter?
Josh Mandel (Jan 14 2020 at 19:47):
What additional channels would be considered as grounds for extensibility,
We've also discussed messaging systems like kafka, and additional web standards like Server-Sent Events.
Josh Mandel (Jan 14 2020 at 19:48):
I think it really should be extensible. Honestly I think our approach to MIME is not super great, because it makes validation harder. @Grahame Grieve do you prefer code
(extensible) to coding
? If so, why?
Josh Mandel (Jan 14 2020 at 19:48):
I don't know of any relevant pre-existing vocabularies.
Josh Mandel (Jan 14 2020 at 19:49):
(other than http://hl7.org/fhir/subscription-channel-type ;-))
Gino Canessa (Jan 14 2020 at 19:50):
I can't find any RFC that matches... the closest thing I can think of is URL protocols, but there would be collisions (in this thread already from rest-hook to GC pub/sub).
Grahame Grieve (Jan 14 2020 at 19:51):
our approach to MIME is not super great, because it makes validation harde
hmm. in what respect? I'm thinking about that and not sure how
Grahame Grieve (Jan 14 2020 at 19:52):
do you prefer code (extensible) to coding
I prefer code over Coding when there's an existing grammar from an RFC to use that saves us from doing definitions, yes. I don't prefer code over coding if we're going to do the definitions
Josh Mandel (Jan 14 2020 at 19:55):
Re: MIME, I just mean that we publish valuests for MIME types liek: http://hl7.org/fhir/valueset-mimetypes.html, which leads to:
Josh Mandel (Jan 14 2020 at 19:56):
This causes developers not to do as much validation as they might if an off-the-shelf valuset was bound to these fields (like we'd presumably want to do for channel types).
Grahame Grieve (Jan 14 2020 at 19:58):
from a methodology view point, we could specify a listed set, but mime types don't actually work like that
Josh Mandel (Jan 14 2020 at 19:59):
I agree, and FHIR's approach to MIME types isn't the wrong call. I'm just saying it's a pain point for developers; I wouldn't want to cause that pain for channel types if we can reasonably avoid it.
nicola (RIO/SS) (Jan 15 2020 at 10:32):
@Gino Canessa explicit timeout is important because otherwise, the server wants to set a timeout, but it does not know which. If it's small - the client may have problems, if it's too long - the server spends resources. It's definitely "recommended by client" timeout which server can override.
Gino Canessa (Jan 15 2020 at 16:05):
@nicola (RIO/SS) Yes, that is what I ended up with in my notes. I don't necessarily agree in practice - from the server side I have a timeout for HTTP connections and I would not change it based on what a client requests. But, I can see the general usefulness and agree that other people are nicer than me :-)
Eric Haas (Feb 19 2020 at 20:55):
Can we flatten out Subscription even more, why do we need channel object. its all max = 1 so could these all be first class elements :
...
channelType (codeable)
endpoint
etc
...
Gino Canessa (Feb 19 2020 at 21:05):
Looks like it works.
- Pro: simpler
- Con: loses logical grouping
Any strong feelings?
Pascal Pfiffner (Feb 19 2020 at 23:05):
I actually like the backbone element that gives logical grouping.
Lloyd McKenzie (Feb 19 2020 at 23:36):
General recommendation in FHIR resource design is to not use grouping unless:
- the group repeats
- it makes sense to mark the group as optional or required as a collection
- the group represents a logical context for extensions
Lloyd McKenzie (Feb 19 2020 at 23:37):
(though I don't think that last bullet is actually written down anywhere...)
Eric Haas (Feb 19 2020 at 23:51):
second bullet makes no sense to me.
group 0..1 and all children 1..1
group 1..1 and all children 0..1
?
Lloyd McKenzie (Feb 20 2020 at 00:10):
Second bullet doesn't place cardinality constraints on the children, it just says that it makes sense to turn them on or off as a group. E.g. CapabilityStatement.software - none of the elements are mandatory, but it does make sense to turn them off as a group if you're dealing with an IG-level CapabilityStatement.
Josh Mandel (Feb 20 2020 at 03:24):
If we wound up getting rid of the grouping, we would probably need to push the name "channel" down from the grouping level into a prefix for each of the individual property names.
Josh Mandel (Feb 20 2020 at 03:24):
I think the grouping is actually pretty useful.
Eric Haas (Feb 20 2020 at 04:34):
I only see channelType as being necessary everything else could stay the same since your context is that channel type. There is no functional reason for nesting only to make it pretty and neat from a modeling perspective. Its a pretty simple resource so I think is make it look more complex than it is
Josh Mandel (Feb 20 2020 at 05:13):
Agreed there is no functional reason for nesting.
Josh Mandel (Feb 20 2020 at 05:15):
Looking back through all the subelement names, I think I agree that channel type is the only really critical one to include the word "channel" in. So I think you've convinced me :-)
Pascal Pfiffner (Feb 27 2020 at 17:30):
Pascal Pfiffner (Feb 27 2020 at 17:30):
Ok fine :slight_smile:
Last updated: Apr 12 2022 at 19:14 UTC