session: clarify ResponseMetaHeader's fields presence - #404
Conversation
92648b9 to
c4e8979
Compare
c4e8979 to
874f28f
Compare
Refs nspcc-dev/neofs-api#404. Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
cthulhu-rider
left a comment
cthulhu-rider
left a comment
There was a problem hiding this comment.
i'd drop as much fields as possible right now and keep only really handled ones. And describe all cases when they are set: there are few of them actually
i think it's also worth noting the behavior for server-side streams w/ multiple response messages like object GET. To me, only 1st message can have meta header
| uint64 epoch = 2 [json_name = "epoch"]; | ||
|
|
||
| // Maximum number of intermediate nodes in the request route | ||
| // Maximum number of intermediate nodes in the request route. Optional. |
There was a problem hiding this comment.
i'd just mark it as deprecated. origin is already marked, and field affects nothing right now and completely senseless to me
There was a problem hiding this comment.
Needs to be checked wrt real use. Reputation service cares about path for example. But if unused can be deprecated.
There was a problem hiding this comment.
no, seems they have never been used. nodes do not restrict anything based on this value. it is also meaningless to me without origin chain. the current reputation service is based on p2p local reviews and public keys
| // Information about the response | ||
| message ResponseMetaHeader { | ||
| // Peer's API version used | ||
| // Server's API version used. Must be attached only if server's version is |
There was a problem hiding this comment.
what meaning could the field have in this case (server < client)?
this is how i see it. If server detects unknown field in the request - it should be either rejected or forwarded. If all fields are known, behavior should be fixed and (minor)version-independent, making fields excessive again. Overall, I mean i dont expect having any "old way" to process fixed request that client should think about. This only complicates and can destabilize the system
and API version is available through node info rpc
There was a problem hiding this comment.
client may change its requests to the server accordingly
| neo.fs.v2.refs.Version version = 1 [json_name = "version"]; | ||
|
|
||
| // Peer's local epoch number | ||
| // Peer's local epoch number. Optional. |
There was a problem hiding this comment.
- net info rpc returns it
- there is more reliable source for it - FS chain
- requirement for the field does not depend on the specific request. Server is free to do as it pleases
so the field is redundant for me. I don't see any point in continuing to support it
i think the same about Optional part of x_headers
There was a problem hiding this comment.
i think in general it can be used somewhere. there were even "caches" (that did not work) in SDK. i think once we are not dependent that much on epochs, we may drop it here too
There was a problem hiding this comment.
Can be deprecated in future. OK as optional for now.
874f28f to
6e60f4c
Compare
|
Added stream adjustments. |
9fbdd34 to
342619d
Compare
Closes #397. Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
342619d to
81ecba7
Compare
Refs nspcc-dev/neofs-api#404. Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
Refs nspcc-dev/neofs-api#404. Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
Refs nspcc-dev/neofs-api#404. Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
Refs nspcc-dev/neofs-api#404. Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
Refs nspcc-dev/neofs-api#404. Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
Refs nspcc-dev/neofs-api#404. Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
Refs nspcc-dev/neofs-api#404. Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
Refs nspcc-dev/neofs-api#404. Signed-off-by: Pavel Karpy <carpawell@nspcc.io>
3 participants
Closes #397.