consul-rust icon indicating copy to clipboard operation
consul-rust copied to clipboard

Implemented /v1/catalog/service/:service with strong types

Open pierresouchay opened this issue 4 years ago • 3 comments

Will allow validating various datastructures (ex: serviceName with '/', Metadata limite keys/size...)

pierresouchay avatar Mar 07 '20 15:03 pierresouchay

Unfortunately the keyword type doesn't introduce a new type, only a type alias. So while this will make code more readable it can lead to some surprises and won't introduce strong types. A good article on this can be found here: https://doc.rust-lang.org/1.8.0/book/type-aliases.html

I wrote a quick example of how you can have data easily move between these type aliases without the compiler stopping you: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=030d70ba1b7660bb76845ce43251d2ee

To get the strong typing we will need to implement each type as a new struct and provide the appropriate guidance to serde.

stusmall avatar Mar 15 '20 21:03 stusmall

@stusmall In the current implementation, I did implement the following real types:

  1. ConsulAddress(String) => used in several fields
  2. ConsulID(String) => used as an alias by ServiceID, NodeID: make sense because IDs are shared the same format, do you want me to create separate types?
  3. ConsulName(String) => used as an alias by ServiceName, NodeName: it is the same thing, ConsulName can be exposed as DNS labels, hence the need for validation at build time (see for instance the current PR in Consul: https://github.com/hashicorp/consul/pull/7450 )

For now, 1, 2 and 3 seem Ok to me as it allows to enforce common semantics.

There are also those aliases:

  • ServiceTags = Vec<String> : For those we could enforce values: it could in theory be enforced to disallow DNS invalid names
  • ServicePort = u16;
  • OptionalServicePort = Option<ServicePort>;

I was wondering if moving slowly for no types to aliases and eventually real types do make sense or if we want directly to use strong types everywhere.

Can you tell me which types you don't want to be aliases?

pierresouchay avatar Mar 24 '20 07:03 pierresouchay

@stusmall Do you have comments regarding https://github.com/stusmall/consul-rust/pull/39#issuecomment-603084048 ?

pierresouchay avatar Apr 10 '20 23:04 pierresouchay