Skip to content

Conversation

@alexey-ivanov-es
Copy link
Contributor

Elasticsearch PR: elastic/elasticsearch#139634

@github-actions
Copy link
Contributor

github-actions bot commented Dec 17, 2025

Following you can find the validation changes against the target branch for the APIs.

API Status Request Response
esql.delete_view ➕ 🟢 1/1 0/0
esql.get_view ➕ 🟢 6/6 0/0
esql.put_view ➕ 🟢 2/2 0/0
project_routing.create ➕ ⚪ Missing test Missing test
project_routing.create_many ➕ ⚪ Missing test Missing test
project_routing.delete ➕ ⚪ Missing test Missing test
project_routing.get ➕ ⚪ Missing test Missing test
project_routing.get_many ➕ ⚪ Missing test Missing test

You can validate these APIs yourself by using the make validate target.

Copy link
Member

@flobernd flobernd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you 🙂 Just a few minor questions/comments.

* under the License.
*/

export class ProjectRoutingExpression {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this class named the same in the Elasticsearch code base? I personally would have used ProjectRouting as the class name since the expression is only a part of it, but if it's the same in Elasticsearch, let's keep it as is 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking at the server code (in serverless) and it seems like every class there is actually called SomethingProjectRoutingExpression, so I'd actually argue all other endpoint classes should be named as closely as possible to what's in the server code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the server code it is SavedProjectRoutingExpression, but I think Saved prefix would be confusing in API

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree for Saved, I'd keep Expression though

Copy link
Member

@flobernd flobernd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks a lot!

@alexey-ivanov-es alexey-ivanov-es merged commit 79a4e6c into main Dec 18, 2025
10 checks passed
@alexey-ivanov-es alexey-ivanov-es deleted the ES-13787 branch December 18, 2025 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

serverless skip-backport This pull request should not be backported specification

4 participants