Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

restlib.DoHTTPRequest requires that the abstraction layers be broken :( #93

Open
ghost opened this issue May 6, 2020 · 4 comments
Open
Labels
P1 Medium priority

Comments

@ghost
Copy link

ghost commented May 6, 2020

For downstream endpoints, there is a bit of a problem with how the request params get passed to the restlib.

sysl generates a req struct which contains the url params and and request body (and presumably the url query params?) but does NOT include any headers.

restlib.DoHTTPRequest() pulls the headers out of the context, and the generated code checks for the required headers. The problem is the service implementation should not care about such details (the fact that downstream is via a REST api should be irrelevant), but the only way to get the header values in is by adding them manually.

The request struct which is generated should contain these header values instead of getting them from the context.

@ghost ghost added the P1 Medium priority label May 6, 2020
@springwiz
Copy link
Contributor

springwiz commented May 7, 2020

Lot of the times the headers get passed from upstream to downstream systems which means it is easier to handle them and pass forward inside context.Context. The context.Context is the single source of truth for request headers which is passed forward from upstream to downstream. So I guess its a design tradeoff between purity and convenience.

@ghost
Copy link
Author

ghost commented May 7, 2020

Sure it's convenient when it is true but it severely hurts when it isnt. I don't see this as a purity:convinience tradeoff at all.

@anzdaddy
Copy link
Member

Different headers need different treatment. Some are pass-through by their very nature (trace id?), others are pass-through in some cases and not others (JWT) and some are actually functional parameters (bad idea, but existing practice).

We should capture the different semantics in Sysl and express them appropriately in codegen.

Some code snippets illustrating the difference between current and desired state would be helpful.

@springwiz
Copy link
Contributor

Productify the custom changes done as part of dcx-bff project. Changes included in the PR https://github.service.anz/dcx/anzapp-bff/pull/810.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Medium priority
Projects
None yet
Development

No branches or pull requests

2 participants