This is more consistent with other endpoints.
Details
Diff Detail
- Repository
- rDSTO Storage manager
- Branch
- origin_visit_get_random-None
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 10182 Build 15105: tox-on-jenkins Jenkins Build 15104: arc lint + arc unit
Event Timeline
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/861/ for more details.
In that case, I prefer an empty dict or an empty list.
Instead of Optional[Dict] or Optional[List], which i found convoluted (especially now that we are adding types)...
Also, like i said on irc, not all endpoints behave as mentioned, there is content_get_metadata which returns a Dict[bytes, List[Dict]].
Anyway, my main concern is that at some point, we should decide whether to continue having those endpoints returns None.
the arguments "all other endpoints are like that" is getting old on me.
Consistency yes, but good consistency.
And I'm not sure we are there.
Note:
it's not limited to that, I also would like we align inputs, stop using generator for some, then list for others, ...
using Optional in the signature makes it explicit that we may return no object.
Also, like i said on irc, not all endpoints behave as mentioned, there is content_get_metadata which returns a Dict[bytes, List[Dict]].
That's different. content_get_metadata returns multiple objects, and there may be 0, 1, or many results. So possibly-empty dicts and list are ok there.
But origin_visit_get_random returns either 0 or 1 result, so it's None for 0 and a non-empty dict for 1.
Maybe it's my FP/ADT background leaking, in that for me, "X = always 1 object", "Option<X> = 0 or 1 object", "container of X = 0 or 1 or many objects". (I'm also disappointed there's generally no way to express "1 or many objects" at the type level; some people tried that in Rust but it's not convincing, eg. https://www.reddit.com/r/rust/comments/8rst01/any_peeps_interested_in_developping_a_fast/ )
Note:
it's not limited to that, I also would like we align inputs, stop using generator for some, then list for others, ...
I'm all for it. If you look at the code of the Cassandra backend, there are a bunch of TODOs about changing names of arguments, but I can't do it without breaking the RPC API (which passes argument names)
Maybe it's my FP or ADT background leaking, in that for me, "X = always 1 object", "Option<X> = 0 or 1 object", "container of X = 0 or 1 or many objects".
great, i see it that way as well.
it's just that as usual not {} or not [] which tends to blur the identity matter...
Also if we stopped using dict but object, that'd be clearer... Optional[OriginVisit] is the type we want here, i think.
I think that's in the pipe ;)
Oh, yes. It will be wonderful to no longer have dicts around anymore, but we're stuck with them for now. Maybe we can start using Model objects at the same time we move to a new RPC framework (@douardda mentioned gRPC)
Build is green
See https://jenkins.softwareheritage.org/job/DSTO/job/tox/867/ for more details.