oapi-codegen icon indicating copy to clipboard operation
oapi-codegen copied to clipboard

Refactor: Pass request context instead of gin.Context in handler template

Open sinyakinilya opened this issue 10 months ago • 6 comments

Refactor: Pass request context instead of gin.Context in handler template

Description:

This PR modifies the handler function template to use ctx.Request.Context() instead of passing ctx directly. This change improves compatibility with middleware that modifies the request context, ensuring better integration with Go's standard context.Context handling.

Why this change?

  • Using gin.Context directly makes it harder to propagate context values through middleware.
  • Switching to ctx.Request.Context() aligns with best practices for handling request-scoped data.
  • This change avoids potential issues when working with custom middleware that relies on context.WithValue.

Impact:

No breaking changes: The function signature remains the same.
Better compatibility with existing Go context patterns.

Checklist:

  • [x] Ran make tidy, make test, make generate, and make lint
  • [x] Verified that existing test cases pass
  • [x] Ensured minimal impact on generated code

Maintainer Notes:
This change is unlikely to cause backward compatibility issues since it only affects the way context is passed within the handler function. Let me know if any additional tests or adjustments are required. 🚀

sinyakinilya avatar Mar 06 '25 16:03 sinyakinilya

This change is unlikely to cause backward compatibility issues since it only affects the way context is passed within the handler function

Is there any chance that this could lead to broken behaviour for folks already using this? (I'm not super familiar with Gin)

jamietanna avatar Apr 24 '25 08:04 jamietanna

This change is unlikely to cause backward compatibility issues since it only affects the way context is passed within the handler function

Is there any chance that this could lead to broken behaviour for folks already using this? (I'm not super familiar with Gin)

Hi @jamietanna

Doesn't look like it should cause any breaks. Currently it's buggy as I was playing around with Otel setup and the trace is setup in gin context by default so only request data is getting captured but not the pgx data as it relies on request context.

Upon changing the ctx to ctx.Request.Context() things are working fine.

vivekkairi avatar May 06 '25 03:05 vivekkairi

This change is unlikely to cause backward compatibility issues since it only affects the way context is passed within the handler function

Is there any chance that this could lead to broken behaviour for folks already using this? (I'm not super familiar with Gin)

Hi @jamietanna

Doesn't look like it should cause any breaks. Currently it's buggy as I was playing around with Otel setup and the trace is setup in gin context by default so only request data is getting captured but not the pgx data as it relies on request context.

Upon changing the ctx to ctx.Request.Context() things are working fine.

Hi @jamietanna

Any ETA on shipping this to prod? We need it for tracing purpose. Right now we have written a go generate to fix the gen.go files.

vivekkairi avatar May 19 '25 06:05 vivekkairi

Thanks for your use of oapi-codegen! We are very appreciative of your interest in improving the project, and that you're engaged with us. However, @-mentioning the maintainers will be very unlikely to help move your issue along, and we do not want to encourage behaviour that leads to the "noisiest" users getting the benefit over folks who are waiting their turn. Please remember that oapi-codegen is for the most part run by volunteers, and we have a request out to companies who use the project to help make it more sustainable, with sponsorship. It will only be that sustained financial support will be able to make it possible to work more efficiently towards user requests. For more info on engaging with the maintainers, see our CONTRIBUTING guide.

github-actions[bot] avatar May 19 '25 06:05 github-actions[bot]

I'll try to look at this soon, but note that tagging maintainers isn't a behaviour we want to encourage - we appreciate there are many issues folks want to see resolved, but if we cater to "the loudest" we don't treat users who are patiently waiting the same

jamietanna avatar May 19 '25 06:05 jamietanna

ping/pong It would be amazing if this gets reviewed ^_^

Evelina-Eevee avatar Oct 21 '25 11:10 Evelina-Eevee