-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
[ML] Remove unused last_data_time member from Job #34262
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pinging @elastic/ml-core |
droberts195
approved these changes
Oct 3, 2018
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
retest this please |
davidkyle
force-pushed
the
duplicate-parsing
branch
from
October 4, 2018 08:14
130a7b8
to
f4bf171
Compare
jasontedor
added a commit
to jasontedor/elasticsearch
that referenced
this pull request
Oct 4, 2018
* master: (25 commits) HLRC: ML Adding get datafeed stats API (elastic#34271) Small fixes to the HLRC watcher documentation. (elastic#34306) Tasks: Document that status is not semvered (elastic#34270) HLRC: ML Add preview datafeed api (elastic#34284) [CI] Fix bogus ScheduleWithFixedDelayTests.testRunnableRunsAtMostOnceAfterCancellation Fix error in documentation for activete watch SCRIPTING: Terms set query expression (elastic#33856) Logging: Drop remaining Settings log ctor (elastic#34149) [ML] Remove unused last_data_time member from Job (elastic#34262) Docs: Allow skipping response assertions (elastic#34240) HLRC: Add activate watch action (elastic#33988) [Security] Multi Index Expression alias wildcard exclusion (elastic#34144) [ML] Label anomalies with multi_bucket_impact (elastic#34233) Document smtp.ssl.trust configuration option (elastic#34275) Support PKCS#11 tokens as keystores and truststores (elastic#34063) Fix sporadic failure in NestedObjectMapperTests [Authz] Allow update settings action for system user (elastic#34030) Replace version with reader cache key in IndicesRequestCache (elastic#34189) [TESTS] Set SO_LINGER and SO_REUSEADDR on the mock socket (elastic#34211) Security: upgrade unboundid ldapsdk to 4.0.8 (elastic#34247) ...
jasontedor
added a commit
to jasontedor/elasticsearch
that referenced
this pull request
Oct 4, 2018
* rename-ccr-stats: (25 commits) HLRC: ML Adding get datafeed stats API (elastic#34271) Small fixes to the HLRC watcher documentation. (elastic#34306) Tasks: Document that status is not semvered (elastic#34270) HLRC: ML Add preview datafeed api (elastic#34284) [CI] Fix bogus ScheduleWithFixedDelayTests.testRunnableRunsAtMostOnceAfterCancellation Fix error in documentation for activete watch SCRIPTING: Terms set query expression (elastic#33856) Logging: Drop remaining Settings log ctor (elastic#34149) [ML] Remove unused last_data_time member from Job (elastic#34262) Docs: Allow skipping response assertions (elastic#34240) HLRC: Add activate watch action (elastic#33988) [Security] Multi Index Expression alias wildcard exclusion (elastic#34144) [ML] Label anomalies with multi_bucket_impact (elastic#34233) Document smtp.ssl.trust configuration option (elastic#34275) Support PKCS#11 tokens as keystores and truststores (elastic#34063) Fix sporadic failure in NestedObjectMapperTests [Authz] Allow update settings action for system user (elastic#34030) Replace version with reader cache key in IndicesRequestCache (elastic#34189) [TESTS] Set SO_LINGER and SO_REUSEADDR on the mock socket (elastic#34211) Security: upgrade unboundid ldapsdk to 4.0.8 (elastic#34247) ...
kcm
pushed a commit
that referenced
this pull request
Oct 30, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Job has the field
last_data_time
I cannot see any use of this member outside of the unit tests beyond checking it is not set when a job is created. I believe this to be a legacy mistake as the field is always null but it is possible the field was set in some earlier version so backwards compatibility must be maintained. The lenient job parsers (hlrc & core) ignore unknown fields so will dispose of the field if set, the strict job parser is only used at job creation and the field was rejected if set at job creation inJob.invalidCreateTimeSettings
so the effect is the same the job will not be created albeit with a different error message.In the UI the last data time value comes from the datacounts.
Additionally I've refactored copy-and-paste date parsing code used in many of the ml pojos