Remove Access-Control-Expose-Headers response header #67
Conversation
As discussed in snok#66, handling CORS should better be left to a specialized middleware.
README.md
Outdated
| CORSMiddleware, | ||
| allow_origins=['*'], | ||
| allow_methods=['*'], | ||
| allow_headers=['X-Request-ID'], |
There was a problem hiding this comment.
@sondrelg, any reason for this to be the default? Just asking out of curiosity, since it differs from Django-GUID, and hasn't been a standard in a long time. I would guess Heroku is your reason? 🤔
Custom proprietary headers have historically been used with an X- prefix, but this convention was deprecated in June 2012
Source: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers
Anyway, nothing we need to change - just curious!
There was a problem hiding this comment.
Think it's the most prevalent choice. See this package tlinhart linked the other day for example: https://github.com/wesdu/fastapi-opentracing/blob/master/fastapi_opentracing/middleware.py#L24
If there was data to support otherwise we might change this now that we're breaking things anyway, but my expectation is that x-request-id is most commonly used. Breaking this particular setting seems pretty impactful too, so think we'd need a pretty good reason to do so 😅
There was a problem hiding this comment.
I read the deprecation notice as well, but in practice, everyone is using X- headers. E.g. Amazon uses X-Amzn-Trace-Id for tracing purposes. So I read the notice more like "now you don't have to use X- headers any more" instead of "you should not use X- headers".
There was a problem hiding this comment.
I'm not saying I'd like it to be changed, was just curios why you opted for it. I remember researching this when I created Django-GUID, so it just stuck with me. 👍
How ever, now that we're talking about this, what does OpenTelemetry use? Could be an idea to match it.
There was a problem hiding this comment.
OpenTelemetry specification talks about traceId as a span attribute. I find this a good generalizing term for both correlation ID and request ID. The first one is most ofter used in a context of async processing where a client sends a message to a broker and receives a correlation ID that the client later uses for polling for a response. The later term, on the other hand, is AFAIK commonly used in a world of web development, APIs and microservices. Most tracing systems (Jaeger, Zipkin, ...) support X-Request-ID header out of the box.
|
Look great at first glance. I'll come back and review properly + release tomorrow most likely 🙇 Thanks again @tlinhart |
As discussed in #66, handling CORS should better be left to a specialized middleware.