rfc41: add new lookup flags #407
Conversation
spec_41.rst
Outdated
| update (2) | ||
| For lookups of R or jobspec, return an updated version which | ||
| has applied updates from the eventlog. |
There was a problem hiding this comment.
Suggestion: rename update to current.
Also, make it clear that the flag has no effect on other keys as opposed to causing an error.
spec_41.rst
Outdated
| decode (1) | ||
| For lookups of R or jobspec, return the field as a decoded | ||
| JSON object instead of a string. |
There was a problem hiding this comment.
The response payload states that all values are returned as strings so it needs an update.
Maybe it would be slightly clearer to say
return the value as a JSON object instead of a JSON object encoded as a JSON string.
The flag name decode feels a bit ambiguous. What about objectify? 🤷
There was a problem hiding this comment.
Would json be explicit enough?
There was a problem hiding this comment.
but a JSON object and a JSON object encoded as a JSON string are both JSON?
Gosh I hate naming things! 😆
There was a problem hiding this comment.
Hm, it seemed clear to me, but if we need to be pedantic, decode or json_decode is probably better than objectify, for which the primary definition has negative connotations.
There was a problem hiding this comment.
I could live with json on second thought. I didn't mean to be pedantic - just trying to come up with an intuitive name and failing. I don't think it matters that much. How about json?
There was a problem hiding this comment.
I also meant to say as long as it is clearly documented, the actual flag name isn't too important to me either.
There was a problem hiding this comment.
I like json_decode, i dunno about just json ... because the returned RPC object is already json?
|
re-pushed, going with |
chu11
force-pushed
the
rfc41_flags
branch
2 times, most recently
from
d89d320
to
553ab65
Compare
spec_41.rst
Outdated
| current (2) | ||
| For lookups of R or jobspec, return the current version which | ||
| has applied respective updates from the eventlog. This flag has | ||
| no effect on other keys. |
There was a problem hiding this comment.
Maybe the eventlog events ought to be explicitly called out? E.g. say
For lookups of R or jobspec, return the current version. The current version SHALL be computed by applying any resource-update or jobspec-update events that have been posted to the job
eventlog, as described in RFC 21.
Still need to update the response description, which states that all returned values are strings.
|
re-pushed, with some tweaks per comments above |
|
removed WIP, I think this RFC and the associated PRs flux-framework/flux-core#5633 and flux-framework/flux-core#5635 are ready for review |
Problem: The job-info.lookup RPC is missing the new json_decode and current flags. Document the new flags.
Problem: The job-info.lookup RPC is missing new flags that are supported.
Document the new flags.
WIP as we discuss flux-framework/flux-core#5633 ("update" vs "updated" as flag name?)
also .. this specific spec change assumes flux-framework/flux-core#5635 would go in too. Could break into two commits.