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.

Sign up for GitHub

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

Jump to bottom

Run TransportGetComponentTemplateAction on local node #116868

Merged
merged 22 commits into from
Dec 23, 2024
Merged

Run TransportGetComponentTemplateAction on local node #116868

merged 22 commits into from
Dec 23, 2024

Conversation

nielsbauman
Copy link
Contributor

@nielsbauman nielsbauman commented Nov 15, 2024 •
edited
Loading

This action solely needs the cluster state, it can run on any node. Additionally, it needs to be cancellable to avoid doing unnecessary work after a client failure or timeout.

The ?local parameter becomes a no-op and is marked as deprecated.

Relates #101805
Relates #107984

@nielsbauman
Run TransportGetComponentTemplateAction on local node
09c73a2 
This action solely needs the cluster state, it can run on any node.
Additionally, it needs to be cancellable to avoid doing unnecessary work
after a client failure or timeout.

The `?local` and `?master_timeout` parameters become a no-op and are
marked as deprecated.

Relates elastic#101805
Relates elastic#107984
@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label v9.0.0 labels Nov 15, 2024
nielsbauman
nielsbauman commented Nov 15, 2024
View reviewed changes
server/src/main/java/org/elasticsearch/action/support/TransportLocalClusterStateAction.java Outdated Show resolved Hide resolved
server/src/main/java/org/elasticsearch/action/support/TransportLocalClusterStateAction.java Outdated

// Remove the BWC support for the deprecated ?master_timeout parameter.
// NOTE: ensure each usage of this method has been deprecated for long enough to remove it.
@UpdateForV10(owner = UpdateForV10.Owner.DISTRIBUTED_COORDINATION)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I marked Distributed as owner as I figured they own this class. I could also (additionally or instead) add the annotation to the REST action.

@nielsbauman nielsbauman added >enhancement :Data Management/Indices APIs APIs to create and manage indices and templates Team:Data Management Meta label for data/management team and removed needs:triage Requires assignment of a team area label labels Nov 15, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@elasticsearchmachine
Copy link
Collaborator

Hi @nielsbauman, I've created a changelog YAML for you.

@nielsbauman
Copy link
Contributor Author

@elasticmachine update branch

@elasticmachine
Copy link
Collaborator

There are no new commits on the base branch.

@nielsbauman
Merge branch 'main' into component-template
91faa7f 
# Conflicts:
#	server/src/main/java/org/elasticsearch/action/support/TransportLocalClusterStateAction.java
@nielsbauman
Make use of new TransportLocalClusterStateAction changes and fix tests
df808dd
@nielsbauman
Merge branch 'main' into component-template
be1a243 
# Conflicts:
#	server/src/main/java/org/elasticsearch/action/admin/indices/template/get/GetComponentTemplateAction.java
@nielsbauman
Use correct owner for update annotation
df06c2f
@nielsbauman
Copy link
Contributor Author

@DaveCTurner could you have a look at this when you have time? I'm not planning on asking for your review for every Data Management API that I'm updating, but this one includes some changes to TransportLocalClusterStateAction. Once this is merged, we'll hopefully be able to use this as an example for the remaining APIs.

@nielsbauman nielsbauman requested a review from dakrone December 18, 2024 13:21
@leemthompo
Copy link
Contributor

run docs-build

@leemthompo leemthompo added the >docs General docs changes label Dec 19, 2024
@elasticsearchmachine elasticsearchmachine added the Team:Docs Meta label for docs team label Dec 19, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

@nielsbauman nielsbauman added >enhancement and removed >enhancement >docs General docs changes labels Dec 19, 2024
@elasticsearchmachine elasticsearchmachine removed the Team:Docs Meta label for docs team label Dec 19, 2024
@elasticsearchmachine
Copy link
Collaborator

Hi @nielsbauman, I've created a changelog YAML for you.

nicktindall
nicktindall approved these changes Dec 19, 2024
View reviewed changes
Copy link
Contributor

@nicktindall nicktindall left a comment

Choose a reason for hiding this comment

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

LGTM

server/src/main/java/org/elasticsearch/rest/RestUtils.java Outdated Show resolved Hide resolved
dakrone
dakrone requested changes Dec 23, 2024
View reviewed changes
Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

The consumer needs to consume the parameter (even if it discards it) when using the rest compat V_8 version. Can you also add a test for this?

server/src/main/java/org/elasticsearch/rest/RestUtils.java Show resolved Hide resolved
nielsbauman
nielsbauman commented Dec 23, 2024
View reviewed changes
...i-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.component_template/10_basic.yml
Comment on lines +193 to +194
- requires:
test_runner_features: ["headers"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figured this test should always pass - even before this change. But please check my reasoning on that.

@nielsbauman nielsbauman requested review from dakrone and removed request for DaveCTurner December 23, 2024 18:55
dakrone
dakrone approved these changes Dec 23, 2024
View reviewed changes
Copy link
Member

@dakrone dakrone 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 Niels!

@nielsbauman nielsbauman enabled auto-merge (squash) December 23, 2024 19:11
@nielsbauman nielsbauman mentioned this pull request Nov 22, 2024
Open
40 tasks
@nielsbauman nielsbauman merged commit 9641c76 into elastic:main Dec 23, 2024
16 checks passed
@nielsbauman nielsbauman deleted the component-template branch December 23, 2024 20:01
navarone-feekery pushed a commit to navarone-feekery/elasticsearch that referenced this pull request Dec 26, 2024
@nielsbauman @navarone-feekery
Run TransportGetComponentTemplateAction on local node (elastic#116868)
a9706c9 
This action solely needs the cluster state, it can run on any node.
Additionally, it needs to be cancellable to avoid doing unnecessary work
after a client failure or timeout.

The `?local` parameter becomes a no-op and is marked as deprecated.

Relates elastic#101805
Relates elastic#107984
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicktindall nicktindall nicktindall approved these changes

@dakrone dakrone dakrone approved these changes

Assignees
No one assigned
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates >enhancement Team:Data Management Meta label for data/management team v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@nielsbauman @elasticsearchmachine @elasticmachine @leemthompo @dakrone @nicktindall