rfc43: support job-list.list response version #488
Conversation
|
|
| (*integer*, OPTIONAL) Specify the current protocol version. If | ||
| jobs will be streamed, this field is required and SHALL be set | ||
| to 1. If jobs will not be streamed, the protocol version can be | ||
| set to 0 or it can be unlisted. | ||
|
|
There was a problem hiding this comment.
Question: seems like you only need the version in the first response, the version should not change in subsequent responses for version >= 1. Should that be called out here?
There was a problem hiding this comment.
Oh, additionally: This could be due to the diff format, but I'm not seeing the name of the new field, i.e. ..object:: version .
There was a problem hiding this comment.
Oh, additionally: This could be due to the diff format, but I'm not seeing the name of the new field, i.e. ..object:: version
Doh! I forgot it! :-)
There was a problem hiding this comment.
Question: seems like you only need the version in the first response, the version should not change in subsequent responses for version >= 1. Should that be called out here?
I elected to just send it in every response, b/c I figure it would be easier / consistent to have it in every response. I dunno, I felt it would be annoying to have to deal with the first response special compared to the rest of them?
That's me of course. No issue making it just the first response. It is extra bytes being sent on every RPC. On the client side, we might be doing "{ ... s?i }, "version", &version") on every rpc_unpack() call anyways.
There was a problem hiding this comment.
Yeah, so in the case above, wouldn't the version get set in the first response, then left alone in subsequent unpacks? (I didn't test it though)
There was a problem hiding this comment.
My $0.02 would be to leave it off the subsequent responses also, to avoid the unnecessary message overhead. There could be a lot of those responses in a big query result.
chu11
force-pushed
the
rfc43_job_list_version
branch
from
266195a to
868d130
Compare
|
|
chu11
force-pushed
the
rfc43_job_list_version
branch
from
868d130 to
9a4c8a0
Compare
Problem: The job-list.list service is forwards compatible, where older non-streaming clients can communicate with newer services. However it is not backwards compatible, where newer streaming clients can operate with older services. Add a version field to the "job-list.list" RPC response. This can inform newer clients on what protocol behavior is expected from both older and newer job-list.list services.
chu11
force-pushed
the
rfc43_job_list_version
branch
from
9a4c8a0 to
01c321a
Compare
|
re-pushed with a minor tweak, saying that "version" is required to be sent in the first response but not in the following responses. |
|
|
1 similar comment
|
|
Merge Queue Status✅ The pull request has been merged at 01c321a This pull request spent 5 seconds in the queue, with no time running CI. Required conditions to merge
|
3 participants
Problem: The job-list.list service is forwards compatible, where older non-streaming clients can communicate with newer services. However it is not backwards compatible, where newer streaming clients can operate with older services.
Add a version field to the "job-list.list" RPC response. This can inform newer clients on what protocol behavior is expected from both older and newer job-list.list services.