Skip to content

Conversation

DiannaHohensee
Copy link
Contributor

@DiannaHohensee DiannaHohensee commented Aug 7, 2025

Pull a Transport*Action out of SnapshotsService
in order to simplify and focus the SnapshotsService
code.

Relates ES-11650


A step of the refactor plan described in #127419

Pull a Transport*Action out of SnapshotsService
in order to simplify and focus the SnapshotsService
code.

Relates ES-11650
@DiannaHohensee DiannaHohensee self-assigned this Aug 7, 2025
@DiannaHohensee DiannaHohensee added >non-issue :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed Coordination Meta label for Distributed Coordination team v9.2.0 labels Aug 7, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

Copy link
Contributor

@JeremyDahlgren JeremyDahlgren left a comment

Choose a reason for hiding this comment

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

Looks like SnapshotResiliencyTests just needs the new action added to the actions map in the TestClusterNode constructor.

@DiannaHohensee DiannaHohensee requested a review from a team as a code owner August 7, 2025 17:24
Copy link
Contributor Author

@DiannaHohensee DiannaHohensee left a comment

Choose a reason for hiding this comment

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

Looks like SnapshotResiliencyTests just needs the new action added to the actions map in the TestClusterNode constructor.

Thanks for taking a look at the test failure, fixed in 46bb887

Copy link
Contributor

@JeremyDahlgren JeremyDahlgren left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticsearchmachine elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Aug 7, 2025
@DiannaHohensee DiannaHohensee added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) and removed serverless-linked Added by automation, don't add manually labels Aug 7, 2025
@DiannaHohensee
Copy link
Contributor Author

Found a stateless action caller: put up https://github.com/elastic/elasticsearch-serverless/pull/4352 to update that caller.

@DiannaHohensee DiannaHohensee added serverless-linked Added by automation, don't add manually and removed auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) labels Aug 7, 2025
@DiannaHohensee DiannaHohensee merged commit 0d57996 into elastic:main Aug 7, 2025
34 checks passed
actions.register(TransportDeleteSnapshotAction.TYPE, TransportDeleteSnapshotAction.class);
actions.register(TransportCreateSnapshotAction.TYPE, TransportCreateSnapshotAction.class);
actions.register(TransportCloneSnapshotAction.TYPE, TransportCloneSnapshotAction.class);
actions.register(TransportUpdateSnapshotStatusAction.TYPE, TransportUpdateSnapshotStatusAction.class);
Copy link
Member

Choose a reason for hiding this comment

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

This is no need to register this action here since it is never invoked by Client. We will not need modify the operator privilege Constants if we don't register the action here.

Comment on lines +2513 to +2516
actions.put(
TransportUpdateSnapshotStatusAction.TYPE,
new TransportUpdateSnapshotStatusAction(transportService, clusterService, threadPool, snapshotsService, actionFilters)
);
Copy link
Member

Choose a reason for hiding this comment

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

Similary, there should be no need to add it to actions. Simply instantiate it should be sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >non-issue serverless-linked Added by automation, don't add manually Team:Distributed Coordination Meta label for Distributed Coordination team v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants