Make deprecation plugin project-aware #130364
Conversation
Updates the majority of the plugin to be aware of multiple projects. The deprecation plugin is excluded in serverless, so in `DeprecationIndexingComponent` we hard-code the default project ID to avoid an unworthy investment of namespacing effort.
nielsbauman
added
>non-issue
:Data Management/Indices APIs
|
Pinging @elastic/es-data-management (Team:Data Management) |
elasticsearchmachine
added
Team:Data Management
PeteGillinElastic
left a comment
PeteGillinElastic
left a comment
There was a problem hiding this comment.
Just one optional comment.
| public List<DeprecationIssue> check(List<TransformConfig> transformConfigs) { | ||
| List<DeprecationIssue> allIssues = new ArrayList<>(); | ||
| CHECKS.forEach(check -> check.apply(clusterState, transformConfigs, allIssues)); | ||
| CHECKS.forEach(check -> check.accept(transformConfigs, allIssues)); |
There was a problem hiding this comment.
Do we feel like the CHECKS abstraction is still pulling its weight here? There's one check hard-coded, we could just call checkTransformSettings directly, and it'd be more readable. If we needed to add more checks in the future, it's not clear to me that adding a method to the list would be better than adding another method call here. Plus, it's not clear to me that the BiConsumer would have the correct signature for a theoretical future check, either. (Presumably, at some point in the past there was a check which consumed ClusterState, hence the redundant parameter. A future check might need that, or might need the ProjectMetadata, but we can't really guess. now.)
There was a problem hiding this comment.
I hear what you're saying. My intuition was that this CHECKS field makes this class more consistent with the other deprecation checkers. But I don't have strong feelings, so I'm fine with calling that method directly.
There was a problem hiding this comment.
On second thought, I think I agree with you that the CHECKS abstraction isn't super valuable, and that goes for the other classes too. I don't think it's worth opening a PR for those other classes, but perhaps I'll remember it if I ever need to touch this code again.
Updates the majority of the plugin to be aware of multiple projects. The deprecation plugin is excluded in serverless, so in `DeprecationIndexingComponent` we hard-code the default project ID to avoid an unworthy investment of namespacing effort.
3 participants
Updates the majority of the plugin to be aware of multiple projects.
The deprecation plugin is excluded in serverless, so in
DeprecationIndexingComponentwe hard-code the default project ID to avoid an unworthy investment of namespacing effort.