Skip to content
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

Adding initial public and internal serverless scopes to Analytics-Geo REST handlers #94038

Merged

Conversation

masseyke
Copy link
Member

#93607 added the ability to run Elasticsearch in "Serverless" mode, where access to REST endpoints could be restricted so that the full Elasticsearch API is not available (since a lot of it does not make sense in Servlerless). By default no endpoints are available, but they can be exposed with ServerlessScope annotations.
This PR follows up on #93607 by adding PUBLIC and INTERNAL annotations to the rest handlers owned by the Analytics/Geo team. There are several rest endpoints still under discussion. This PR does not label those, so they remain unavailable in Serverless mode.

@masseyke masseyke added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.8.0 labels Feb 22, 2023
@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label and removed Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) labels Feb 22, 2023
@masseyke masseyke added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) and removed needs:triage Requires assignment of a team area label labels Feb 22, 2023
@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label and removed Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) labels Feb 22, 2023
@thecoop thecoop added :Analytics/Geo Indexing, search aggregations of geo points and shapes and removed needs:triage Requires assignment of a team area label labels Mar 8, 2023
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Mar 8, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

@@ -19,6 +21,7 @@

import static org.elasticsearch.rest.RestRequest.Method.POST;

@ServerlessScope(Scope.PUBLIC)
Copy link
Member

Choose a reason for hiding this comment

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

I wonder whether this should be a public api in server less.
I expect real downsampling usage to be via ilm or via the future data lifecycle mechanism in stateless/serverless.
So maybe make this an internal api? I suspect it easier to make an api public versus changing an api from public to internal?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah we'll likely never be able to take back a public one (after this actually goes live, that is). Do we even need it as internal? I assume ILM is hitting it through transport, not REST, right?

Copy link
Member

Choose a reason for hiding this comment

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

Right, ILM is hitting the the api via transport. But maybe we may want this rest api accessible for debugging purposes. So let's make it internal?

@masseyke masseyke requested a review from martijnvg March 21, 2023 21:02
Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

LGTM

@masseyke masseyke merged commit ea218dd into elastic:main Mar 22, 2023
@masseyke masseyke deleted the feature/analytics-geo-serverless-scopes branch March 22, 2023 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants