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

Refactor data stream lifecycle to use the template paradigm #124593

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

gmarouli
Copy link
Contributor

In this PR we migrate the DataStreamLifecycle configuration to the new "template" framework we introduced in #117357. Effectively, we split the code that is used by the data stream and the templates. This is better for the following reasons:

  • The explicit nulls are scoped only in the template code and will not leak to the data stream (any more).
  • The explicit nulls are configured in a predicable way using the ResettableValue and can be composed easier as well.
  • In a follow up PR when it's time to incorporate the lifecycle under the failure store, it will be relatively easy since the failure store config already uses this template framework.
  • We removed dedicated wrapper classes that where necessary to represent the explicit nulls.

@gmarouli gmarouli added >refactoring :Data Management/Data streams Data streams and their lifecycles labels Mar 11, 2025
@gmarouli gmarouli requested a review from jbaiera March 11, 2025 20:00
@gmarouli gmarouli added auto-backport Automatically create backport pull requests when merged v8.19.0 labels Mar 11, 2025
@elasticsearchmachine elasticsearchmachine added Team:Data Management Meta label for data/management team v9.1.0 labels Mar 11, 2025
@elasticsearchmachine
Copy link
Collaborator

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

@gmarouli
Copy link
Contributor Author

I will work on the CI in the meantime, but I wanted to open it already for a review because it's quite a big.

}

private final boolean enabled;
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 have been going back and forth about this one, should we keep it boolean or should we convert it to Boolean.

When it comes to failure store we had a specific use case where we enable it via a cluster setting. Here, we do not have such a use case yet, so I am leaning towards keep it as is. Any thoughts?

cc @dakrone

@elasticsearchmachine elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged :Data Management/Data streams Data streams and their lifecycles >refactoring serverless-linked Added by automation, don't add manually Team:Data Management Meta label for data/management team v8.19.0 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants