Always allow using empty objects to close a streaming method#45
Always allow using empty objects to close a streaming method#45daandemeyer wants to merge 1 commit into
Conversation
Currently in systemd, when we have a streaming method call that has nothing to return, we're forced to return an error as we have to reply something and we can't reply an empty object as this would not pass IDL validation. Let's extend the spec to allow returning an empty object to signal the end of a streaming method. This will allow us to return an empty object instead of an error to signal the end of a streaming method.
Implements varlink/varlink.github.io#45. In the future, we can make use of this to avoid having to send errors when there is nothing to stream. It also simplifies the case when there is stuff to stream, as we don't have to keep track anymore of the previous entry to know when to use sd_varlink_reply() or sd_varlink_notify(). We can't start using this behavior right away until we can assume client libraries implement the new addition to the spec. We'll probably also only be able to use this when implementing new varlink methods as we need to consider backwards compat for the old ones.
Implements varlink/varlink.github.io#45. In the future, we can make use of this to avoid having to send errors when there is nothing to stream. It also simplifies the case when there is stuff to stream, as we don't have to keep track anymore of the previous entry to know when to use sd_varlink_reply() or sd_varlink_notify(). We can't start using this behavior right away until we can assume client libraries implement the new addition to the spec. We'll probably also only be able to use this when implementing new varlink methods as we need to consider backwards compat for the old ones.
Implements varlink/varlink.github.io#45. In the future, we can make use of this to avoid having to send errors when there is nothing to stream. It also simplifies the case when there is stuff to stream, as we don't have to keep track anymore of the previous entry to know when to use sd_varlink_reply() or sd_varlink_notify(). We can't start using this behavior right away until we can assume client libraries implement the new addition to the spec. We'll probably also only be able to use this when implementing new varlink methods as we need to consider backwards compat for the old ones.
Implements varlink/varlink.github.io#45. In the future, we can make use of this to avoid having to send errors when there is nothing to stream. It also simplifies the case when there is stuff to stream, as we don't have to keep track anymore of the previous entry to know when to use sd_varlink_reply() or sd_varlink_notify(). We can't start using this behavior right away until we can assume client libraries implement the new addition to the spec. We'll probably also only be able to use this when implementing new varlink methods as we need to consider backwards compat for the old ones.
|
Can you explain why this is desirable? why doesn't the current logic suffice? at least systemd's apis all happily return a "notfound" error if you ask for a list of things that is empty, and there's a lot of tooling to nicely deal with that (for example |
|
@poettering Because not having anything to return is not always an error. It's equivalent to returning ENOENT or ESRCH every time you list something in systemd if there is nothing. A good example is listing transfers in importd. In no way is it an error if there are no ongoing transfers at some point in time. Yet there is no other way in varlink than an error to report no ongoing transfers. If you ask me this is a deficiency in the protocol. |
Implements varlink/varlink.github.io#45. In the future, we can make use of this to avoid having to send errors when there is nothing to stream. It also simplifies the case when there is stuff to stream, as we don't have to keep track anymore of the previous entry to know when to use sd_varlink_reply() or sd_varlink_notify(). We can't start using this behavior right away until we can assume client libraries implement the new addition to the spec. We'll probably also only be able to use this when implementing new varlink methods as we need to consider backwards compat for the old ones.
Implements varlink/varlink.github.io#45. In the future, we can make use of this to avoid having to send errors when there is nothing to stream. It also simplifies the case when there is stuff to stream, as we don't have to keep track anymore of the previous entry to know when to use sd_varlink_reply() or sd_varlink_notify(). We can't start using this behavior right away until we can assume client libraries implement the new addition to the spec. We'll probably also only be able to use this when implementing new varlink methods as we need to consider backwards compat for the old ones.
Implements varlink/varlink.github.io#45. In the future, we can make use of this to avoid having to send errors when there is nothing to stream. It also simplifies the case when there is stuff to stream, as we don't have to keep track anymore of the previous entry to know when to use sd_varlink_reply() or sd_varlink_notify(). We can't start using this behavior right away until we can assume client libraries implement the new addition to the spec. We'll probably also only be able to use this when implementing new varlink methods as we need to consider backwards compat for the old ones.
|
To me this smells a bit like bike shedding and I dont think that an empty object is a good indicator for EOF. After all there are plenty functions that return an empty object on success, and thus making this mean more than just "success but got nothing to say for now" is quite a compat breakage. i mean, in plenty of subscription apis we probably want that the first reply just indicates "the subscription is in place now, but i have nothing to report yet". such a reply would be much better encoded in an empty object, hence using it for EOF is really problematic. Also, I think the ship has simply sailed already on this, there's a lot of code in place already that just uses "NoSuchObject" errors to indicate empty enumeration. |
|
The fact that we messed up a bunch of apis doesn't mean we can't fix future APIs. It doesn't have to be an empty object, I'm happy to add a dedicated sentinel object, but we should have something that isn't an error. |
To me this sounds like one additional reason to introduce a reply type that is exempted from usual IDL nullable checks ("out-of-band"?). Since such replies are only used as indicators, and for real contents you want to have precise notations of whether a field is nullable or not. The current API just forces us to make everything nullable in order to encode such info... |
|
so I am a bit concerned about the compat story, i.e. having a completely different marker for EOR than we previously had sounds kinda awful to me. Hence what about one of these two compromises:
We could possibly even do the combination of both: the EOR would then be assumed in one of three cases:
I like the |
|
But that said, I wouldn' change anything from the status quo... the userdb example is a good reason why I think the current behaviour is not so bad: it provides a nice hint why we return nothing. |
3 participants
Currently in systemd, when we have a streaming method call that has nothing to return, we're forced to return an error as we have to reply something and we can't reply an empty object as this would not pass IDL validation.
Let's extend the spec to allow returning an empty object to signal the end of a streaming method. This will allow us to return an empty object instead of an error to signal the end of a streaming method.