-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
Add a basic deprecation warning that the JSON format is changing in v9 #114739
base: 8.x
Are you sure you want to change the base?
Conversation
Pinging @elastic/es-core-infra (Team:Core/Infra) |
I'm not sure if this needs to be a critical (as things are definitely changing), or a warning, as the user can't do anything about it until they upgrade. Do we need to include a link to more detailed docs (which don't really exist, apart from the breaking changelog?) |
That seems bad to me. Certainly this can't be critical because critical means "must be addressed before you upgrade". But could we have a way for users to opt-in to the new format ahead of time? Should we fall back to the old format in v9 if the user requests |
This is the behaviour as agreed in the BwC discussion... |
Ok I see, I wasn't in that discussion 😄 Can we tell folks to opt-in to the future behaviour now by removing |
That will cause detailed error information to be output, which is probably not what they want (as they set the option in the first place) |
Sure but we've got to do something here, we can't just expect folks to upgrade and suck it up if their clients can't handle the new format caused by the upgrade. Either we need to help them fix their clients before upgrading or else we need to preserve the v8 error format in v9. |
That's what the decision of the BwC committee was - there's very little use of this option, so any attempt at pro-active mitigation is disproportionate. You can re-open the discussion with BwC if you want? |
That's not my reading of the decision. There was no mention of this change affecting clients that requested v8 compatibility in v9, and our default position for that case would be that such clients see no change in behaviour. Would you ask the BwC committee to clarify whether they intended to make this change in v8-compat mode too? |
I agree with Dave's concern. The point of rest api compatibility is to bridge these exact gaps, changes across majors. I think we should maintain the existing format when the v8 header is used. |
I've added BwC API outputs to #90529, and tweaked the warning here accordingly |
"The JSON format of non-detailed errors will change in Elasticsearch 9.0 to match the JSON structure" | ||
+ " used for detailed errors. To keep using the existing format, use the V8 REST API." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need the same deprecation warning in the v9 code too (when hitting this path in v8 compat mode).
I also don't think we can have a critical warning without any way for folks to resolve it before upgrading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed it to a warning, and will copy these changes to the v9 PR
Warn that #90529 changes the json format when detailed errors are turned off