Skip to content

Remove LegacyCTRAL from AutoCreateAction#84170

Merged
DaveCTurner merged 5 commits intoelastic:masterfrom
DaveCTurner:2022-02-18-auto-create-action-LegacyCTRAL
Mar 2, 2022
Merged

Remove LegacyCTRAL from AutoCreateAction#84170
DaveCTurner merged 5 commits intoelastic:masterfrom
DaveCTurner:2022-02-18-auto-create-action-LegacyCTRAL

Conversation

@DaveCTurner
Copy link
Copy Markdown
Member

@DaveCTurner DaveCTurner commented Feb 18, 2022

Relates #83784

@DaveCTurner DaveCTurner added WIP :Data Management/Indices APIs DO NOT USE. Use ":Distributed/Indices APIs" or ":StorageEngine/Templates" instead. v8.2.0 labels Feb 18, 2022
@DaveCTurner DaveCTurner force-pushed the 2022-02-18-auto-create-action-LegacyCTRAL branch from 999f845 to 7e2ff44 Compare February 22, 2022 16:53
@DaveCTurner DaveCTurner marked this pull request as ready for review February 22, 2022 16:54
@elasticmachine elasticmachine added the Team:Data Management (obsolete) DO NOT USE. This team no longer exists. label Feb 22, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

@DaveCTurner DaveCTurner requested review from joegallo, original-brownbear and probakowski and removed request for probakowski February 22, 2022 16:54
};
}

ClusterState execute(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is it worth a comment on the implied contract w.r.t. returning a clusterState, calling success/failure on the taskContext, and updating successfulRequests? It looks to me like you're doing all the juggling just fine, but some future developer might be happy to add a new return case and think that just satisfying the compiler here is sufficient.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, good point. I added a comment in 6be0514 and also an assertion to enforce it.

Copy link
Copy Markdown
Contributor

@joegallo joegallo left a comment

Choose a reason for hiding this comment

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

LGTM, I'd like to see a comment about it, but I'm not going to block on that.

@DaveCTurner DaveCTurner merged commit 1e338c6 into elastic:master Mar 2, 2022
@DaveCTurner DaveCTurner deleted the 2022-02-18-auto-create-action-LegacyCTRAL branch March 2, 2022 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Data Management/Indices APIs DO NOT USE. Use ":Distributed/Indices APIs" or ":StorageEngine/Templates" instead. >refactoring Team:Data Management (obsolete) DO NOT USE. This team no longer exists. v8.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants