Add limit= to get methods, make their signatures more uniform

[mk-fg]

• Adding one missing request parameters, one key at a time...

• Ordered keys to be more like rpc.proto, maybe e.g. get_range(self, range_start, range_end, *, ...) in all sigs can be used to avoid such ordering/additions ever breaking code in the future, but guessing it's not an issue atm anyway.

• Added docstrings for couple parameters where they were missing, but dunno how these should look for sort_order and such enums, so skipped these here.

• get_all() was missing type annotations.

[martyanov]

Thanks! I planned to wrap the rpc enums in custom enum types, like one from watch commands.

[martyanov]

Please see https://github.com/martyanov/aetcd/blob/master/aetcd/rtypes.py#L257 for example. I personally don't like the string literals here, pretty much easy to abuse.

[mk-fg]

Can probably be fixed in a follow-up commit or PR.

[mk-fg]

Added new enums for ordering in the second commit, a couple notes on those:

• Made them comparable to case-insensitive strings (via shared _CaseInsensitiveStringEnum class).

  Idea is to allow people to type 'key' instead of 'KEY' or aetcd.rtypes.RangeSortTarget.KEY. Only downside seem to be having that small superclass and maybe visual inconsistency that it allows, but maybe allowing to avoid HARD_TO_READ_AND_TYPE UPPERCASE is worth it. Feel free to just drop it though - should be as easy as removing that superclass and adjusting tests.

• sort_order/target no longer accept None, only proper enum values.

  Seems a bit weird to have one special value there, which is then always translated to enum value under the hood, i.e. not actually optional in any way. In sort_target it's also translated into 'key', which seems a bit counter-intuitive, so maybe better to just ask for proper enum match in all cases?

  This is incompatibility, but same as mentioned in other PRs, maybe ok to introduce at this point. Should be easy to revert by using typing.Optional for types and something like rtypes.RangeSortOrder(sort_order or 'none').name in _build_get_range_request.

• Haven't build docs to test how new descriptions look there, but they are very similar to existing ones, so barring any typos, should probably be fine.

[martyanov]

Why not just SortOrder and SortTarget as names for the enums?

Not sure if I like the _CaseInsensitiveStringEnum, I prefer to be more strict and explicit. :) Breaking the API at the moment is not an issue at all, alpha status of the prject is reflected in its version and project meta.

I'll take a closer look. Thanks!

[martyanov]

A little follow up. If you make the enums importable via aetcd, as all other rtypes types, then it's just a aetcd.SortTarget.KEY. Personally I'm fine with this.

[mk-fg]

Why not just SortOrder and SortTarget as names for the enums?

They are only relevant for RangeRequest and translated to RangeRequest.SortOrder, so seemed fitting to namespace them like that, similar to how EventKind is not just Kind, but obviously doesn't really matter.

[mk-fg]

Yeah, I think with strings I was also a bit influenced by how EventKind is halfway str + enum type. So seemed to be best to either make them all abstract enums not in any way comparable to strings (as that leads to typos which python can't check), or - if they are string - make them human-friendly strings.

Guess if enums are fine, should get rid of all "str" in there and only ever use/allow enums.

[martyanov]

Yep, but only the enums should be explicitly imported, as other types return implicitly as operation results, so they only needed may be for type annotations. I think the short enum names are self descriptive enough, readability counts. And we dont have many of them in proto definitions.

[martyanov]

Enum-only variant sounds good to me.

[mk-fg]

EventKind, SortOrder, SortTarget should be all auto-value enums now, with None or str passed to sort_order/target options raising ValueError.