Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Compendium Client is missing required query param for storing Protocols #118

Open
rlmark opened this issue Dec 11, 2019 · 5 comments
Open

Comments

@rlmark
Copy link
Contributor

rlmark commented Dec 11, 2019

Hello again! As part of learning generally about Compendium to support the development efforts (https://github.com/47deg/marlow/issues/262) I believe the Compendium Client has drifted out of sync with the server code a tiny bit.

The Compendium server route requires a query param indicating the IDL format for POST'ing new protocols for storage. A valid route looks something like this: http://0.0.0.0:8080/v0/protocol/identifier1?idlName=avro

The route that the client appears to call omits the idlName section, resulting in 404's. Perhaps we could update the client with this minor change and add client support of the new route for schema transformations defined in the Compendium Server.

@juanpedromoreno
Copy link
Member

As compendium has a binary dependency with skeuomorph, I'm proposing to use the open-api derivation feature to derive the compendium client where it's needed, for example:

https://github.com/higherkindness/mu/blob/master/modules/idlgen/core/src/main/scala/higherkindness/mu/rpc/idlgen/openapi/OpenApiSrcGenerator.scala

In other words, let's define the swagger (open-api) specs and derive the client from there so we won't have any binary client that might cause binary problems in the future.

The compendium example where you are working on @rlmark , or even sbt-compendium would use the same approach for deriving the Compendium Http Client at compile time.

@juanpedromoreno
Copy link
Member

For reference, this is an example of open-api code generation from swagger specs through mu: https://github.com/47deg/petstore4s

@JesusMtnez
Copy link
Member

JesusMtnez commented Dec 12, 2019

And how is sbt-compendium or any other project that requires a compendium client going to obtain the specification for the client so it could be generated using skeuomorph? I can only think they should have the json or yaml file in the project.

Also, that would require the project to have a dependency on skeuomorph, right? Just to generate a client for the service/app.

I'm not completely sure it is the best approach to expose a client 🤔

@pepegar @pirita @rlmark @bilki opinions about this?

@JesusMtnez
Copy link
Member

One possible solution here is keeping the client module, but instead of writing the client ourselves, using skeuomorph to derive the client code and just publish that artifact 🤔

One drawback I find here is that we will not control the functions the API offers, so the client could not be extended beyond the HTTP API definition. I think we should agree on that before moving on with this idea.

On the other hand, doing that would remove the need to write the client by ourselves, but instead just derive the client and maybe add some testing to check it before releasing it 🤔

Just trying to think out loud, but please, let me know your opinions on this matter 😄

@rlmark
Copy link
Contributor Author

rlmark commented Dec 12, 2019

Ah, that's an interesting idea @juanpedromoreno! It would reduce the risk of the client and server getting out of sync, provided we remembered to update the OpenApi yaml/json files when we made server changes. Maybe an integration test or CI build step could aid us in that process. (Just as a thought, we could go "all in" on the schema-driven solution and generate the server routes too, but I'm not sure about that just yet.)

@JesusMtnez I like your second point about publishing a separate client module which contains the programmatically generated client artifact. That way the consumers of the client wouldn't have to care about the process of generating the code.

I also see your point about this tightly coupling the client to the http api specification. That's definitely something we should all discuss. I could see features like client-side retry policies being harder to add in programmatically generated code.

I haven't played around with the Skeuomorph OpenAPI code generation feature yet, but I'm assuming it can handle things that are on the Compendium backlog for the future, like support for the headers needed for Authentication/Authorization. 🤔

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants