-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Use deprecated object to deprecate synced flush API #51906
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
Conversation
Relates: #50835 This commit updates the synced flush REST API to use the deprecated object to deprecate the whole API.
It would be ideal if this were included in 7.6.0, to allow generators to pick up for 7.6.0 clients. |
}, | ||
"deprecated" : { | ||
"version" : "7.6.0", | ||
"description" : "Synced flush is deprecated and will be removed in 8.0. Use flush instead" |
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.
Missing period for the second sentence? :)
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.
@russcam In other places (like the update_transform JSON spec), the deprecated
object was added at the paths
objects.
Github won't let me comment there, but the url
object would look something like this:
"url":{
"paths":[
{
"path":"/_flush/synced",
"methods":[
"POST",
"GET"
],
"deprecated":{
"version":"7.6.0",
"description":"[/_flush/synced] is deprecated and will be removed in 8.0. Use [/_flush] instead."
}
},
{
"path":"/{index}/_flush/synced",
"methods":[
"POST",
"GET"
],
"parts":{
"index":{
"type":"list",
"description":"A comma-separated list of index names; use `_all` or empty string for all indices"
},
"deprecated":{
"version":"7.6.0",
"description":"[/{index}/_flush/synced] is deprecated and will be removed in 8.0. Use [/{index}/_flush] instead."
}
}
}
]
},
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.
Thanks @jrodewig. I think the deprecation should be applied at the top level for this and for transforms, otherwise the deprecation object needs to be repeated for each path, which seems superfluous. Based on #51906 (comment) though, it looks like deprecated isn't implemented at this level currently.
The CI seems to fail with:
Perhaps the Java runner doesn't support the field yet? |
Pinging @elastic/es-docs (>docs) |
Pinging @elastic/es-distributed (:Distributed/Engine) |
@russcam are you still wanting to progress this PR or should it be closed? |
I'll close this and reopen a new PR with some changes from my fork |
Relates: #50835
This commit updates the synced flush REST API to use the deprecated object to deprecate the whole API. Client generators can pick up the deprecated object to apply language specific deprecation warnings.