Skip to content

feat: allow round trip middleware per operation#156

Closed
pseudo-su wants to merge 4 commits intooapi-codegen:mainfrom
pseudo-su:feature/allow-round-trip-middleware-per-operation
Closed

feat: allow round trip middleware per operation#156
pseudo-su wants to merge 4 commits intooapi-codegen:mainfrom
pseudo-su:feature/allow-round-trip-middleware-per-operation

Conversation

@pseudo-su
Copy link
Copy Markdown
Contributor

@pseudo-su pseudo-su commented Mar 16, 2020

Reference implementation to cater to the use-cases I described in #150

Notes

Fixes #150

@StevenACoffman
Copy link
Copy Markdown
Contributor

This is exactly the use case I have, so I would love to see a complete example in your pseudo-su /golang-service-template repo

@pseudo-su
Copy link
Copy Markdown
Contributor Author

@StevenACoffman I haven't forgotten about this, you make a good point about examples... I might actually add some more tests cases to this PR that would also help somewhat in capturing examples.

I have a few open PRs for oapi-codegen that may or may not get merged. I was trying to avoid it because I'm lazy but I might merge them all into my forked repo and add a branch to pseudo-su /golang-service-template with examples.

My preference was to get things merged (where possible) before spending too much time messing about with a temporary fork.

Copy link
Copy Markdown
Contributor

@StevenACoffman StevenACoffman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is ready to merge with this minor case change.

@StevenACoffman
Copy link
Copy Markdown
Contributor

@deepmap-marcinr This one is ready too!

John Stableford and others added 3 commits April 2, 2020 09:52
refactor: fix case

Co-Authored-By: Steve Coffman <StevenACoffman@users.noreply.github.com>
@pseudo-su pseudo-su force-pushed the feature/allow-round-trip-middleware-per-operation branch from 48a255b to 474f343 Compare April 1, 2020 22:53
@StevenACoffman
Copy link
Copy Markdown
Contributor

@pseudo-su Hmmm, I think your rebase you need to regenerate because now there's some test failures.

@pseudo-su
Copy link
Copy Markdown
Contributor Author

pseudo-su commented Apr 1, 2020

I need to fix some things and add those example/tests I mentioned, working on it now

@pseudo-su
Copy link
Copy Markdown
Contributor Author

pseudo-su commented Apr 2, 2020

Ok @StevenACoffman three main things changed and this should be good now.

  • joinMiddleware had an issue when passing an empty list of middleware to join (picked up by the new tests from master 🎉 ).
  • I've added tests in internal/test/client/client_test.go that have scenarios where WithSharedRoundTripMiddleware and WithRoundTripMiddleware are used with a mock "interceptor".
  • I fixed the spec in internal/test/client/client.yaml because the responses were missing the content field inbetween the status code and the content-type

EG it was this

responses:
  200:
    application/json:

instead of

responses:
  200:
    content:
      application/json:

@StevenACoffman
Copy link
Copy Markdown
Contributor

Thanks, yes, this works for us in our client. Nice improvement! @deepmap-marcinr Ready for you!

@batazor
Copy link
Copy Markdown

batazor commented Jan 3, 2024

What is the status of this PR? Is there any other way to set custom behavior?

@StevenACoffman @pseudo-su

kodiakhq bot pushed a commit to cloudquery/cloudquery-api-go that referenced this pull request Mar 28, 2024
Looks like this is the only way to inject the retry client, there's also oapi-codegen/oapi-codegen#150 which is in progress via oapi-codegen/oapi-codegen#156
@DAcodedBEAT
Copy link
Copy Markdown

@oapi-codegen any reason why this wasn't approved/merged (aside from the current merge conflicts)?

@pseudo-su pseudo-su closed this Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature proposal: WithRoundTripMiddlewares and WithSharedRoundTripMiddleware

4 participants