disneystreaming / smithy4s

https://disneystreaming.github.io/smithy4s/
Other
347 stars 69 forks source link

Include uri template string in HttpEndpoint #47

Open kubukoz opened 2 years ago

kubukoz commented 2 years ago

Currently, HttpEndpoint contains all the information from the Http trait, but to get the path template back, one needs to manually stringify the path segments. I think it would be valuable for tooling authors if this was another method in HttpEndpoint.

Baccata commented 2 years ago

I'm not against it, but can you elaborate a little more with an example usecase ?

kubukoz commented 2 years ago

Path templates can be useful as documentation, e.g. in a CLI:

image

or an editor integration:

image

Currently in both I'm relying on the presence of the trait on the endpoint :)

Baccata commented 2 years ago

Objectively speaking, I'd think that the reliance on the hint is more idiomatic. For instance, in the case of a CLI, the presence of http paths in the help is icing on the cake rather than crucial information : a CLI front-end could talk to literally any backend.

Moreover, if we are to split the http package in a separate module like I'm intending to, the HttpEndpoint construct would move, and I presume that's not a dependency that a CLI frontend would need.

That being said, maybe we could wrap the list of path segments in a case class and recover the template rendering there. I've been feeling a little bit weird about just exposing the List as such.

kubukoz commented 2 years ago

yeah, that could work.

In general I agree about the point that the CLI doesn't need to know about the details, but I wanted to have a way (whether it's more built in or supported by an interop module is another question) to show protocol-specific documentation as well :)

sbly commented 2 years ago

Hello, I think this has been completed? I see a def path: List[PathSegment] method in HttpEndpoint

kubukoz commented 2 years ago

yeah, that's this part:

but to get the path template back, one needs to manually stringify the path segments

so it was available at the time I created the feature. I'm keeping this open because we still don't have a separate HTTP module.