[core.http] Add warning header to deprecated endpoints#205926
[core.http] Add warning header to deprecated endpoints#205926jesuswr merged 16 commits intoelastic:mainfrom
Conversation
jesuswr
added
Feature:http
Team:Core
|
/ci |
1 similar comment
|
/ci |
jesuswr
force-pushed
the
add-warning-header-to-deprecated-endpoints
branch
from
3f3e276 to
e37ee13
Compare
|
/ci |
1 similar comment
|
/ci |
|
Pinging @elastic/kibana-core (Team:Core) |
jloleysens
left a comment
jloleysens
left a comment
There was a problem hiding this comment.
Overall looks good! Left a couple of comments that I'd like to get your thoughts on before merging, no major blockers! Ran a simple local test and works as expected.
Great work @jesuswr !
There was a problem hiding this comment.
Not sure, but it may be worth adding some kind of truncation mechanism here. I'm not exactly sure how long these might get. Maybe something like 255 chars?
There was a problem hiding this comment.
Nit: maybe it's time to pass down env: this.env instead?
There was a problem hiding this comment.
I was thinking about that, specially if we want to add the buildSha in the warning eventually. Will change it 👍
There was a problem hiding this comment.
This does make me wonder if the Kibana response object can rather be given the deprecation info context so that there is only one middleware that injects the warning header 🤔
Don't want to overcomplicate things though, and I think you're test coverage has all paths covered well!
jesuswr
force-pushed
the
add-warning-header-to-deprecated-endpoints
branch
from
d31ba4b to
057833e
Compare
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
| /** | ||
| * The description message to be displayed for the deprecation. | ||
| * This will also appear in the '299 Kibana-{version} {message}' header warning when someone calls the route. | ||
| * Keep the message concise to avoid long header values. It is recommended to keep the message under 255 characters. |
There was a problem hiding this comment.
@jloleysens added this comment to address what you mentioned about long header values
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Unknown metric groupsAPI count
History
cc @jesuswr |
## Summary resolves elastic#105692 This PR adds a pre response handler that sets a warning header if the requested endpoint is deprecated. ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss. Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
4 participants
Summary
resolves #105692
This PR adds a pre response handler that sets a warning header if the requested endpoint is deprecated.
Checklist
Check the PR satisfies following conditions.
Reviewers should verify this PR satisfies this list as well.
release_note:*label is applied per the guidelinesIdentify risks
Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.
Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.