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

Move responsability to add required headers from clients to the transport client #18

Open
lilgallon opened this issue Nov 2, 2023 · 2 comments
Labels
enhancement New feature or request module: transport

Comments

@lilgallon
Copy link
Member

lilgallon commented Nov 2, 2023

this is not really part of the scope of this PR, but adding those parameters to the withRequiredHeaders makes it more obvious: it's tedious and error prone to have all the clients call this method to add the headers. I think we should move the responsibility to the transportClient to add the required headers in the send method. But I tihnk we can move forward with this PR and address this point in another PR, as well as the review of how authentication and tokens are used / named.

Originally posted by @xhanin in #14 (comment)


It concerns

  • .withRequiredHeader()
  • .authenticate()
@atschabu
Copy link
Contributor

As this requires an object holding a reference to the PartnerRepo and partnerUrl I can see two simple ways of making this work.

a) making TransportClient abstract, and implement send to add these things before calling a doSend that needs to be implemented by actual implementation. Migration effort would be small, and extensible, as implementation could still override existing methods

b) have a "middleware" that implements send method and wraps the actual transport client

@lilgallon
Copy link
Member Author

lilgallon commented Jan 11, 2024

The a) approach would be nice. It's important to be extensible on this part

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request module: transport
Projects
None yet
Development

No branches or pull requests

2 participants