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

Removes two unused AnalysisConfig options #35645

Conversation

benwtrent
Copy link
Member

result_finalization_window is pretty much unused, and over time has become broken due to other features. Consequently, it and overlapping_buckets are being removed as options for AnalysisConfig.

Since these were undocumented and largely unfinished/broken options, removal is acceptable and helps simplify much of the downstream ml-cpp code.

Relates to elastic/ml-cpp#302

@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

Looks good apart from a couple of questionable removals

Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

@benwtrent benwtrent merged commit 6d4a3f8 into elastic:master Nov 19, 2018
@benwtrent benwtrent deleted the feature/ml-remove-result_finalization_window branch November 19, 2018 14:29
benwtrent added a commit that referenced this pull request Nov 19, 2018
* ML: Removing result_finalization_window && overlapping_buckets

* Reverting bad method deletions

* Setting to current before backport to try and get a green build

* fixing testBuildAutodetectCommand test
benwtrent added a commit that referenced this pull request Nov 19, 2018
benwtrent added a commit to benwtrent/elasticsearch that referenced this pull request Nov 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants