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

[ML] Remove unused last_data_time member from Job #34262

Merged
merged 2 commits into from
Oct 4, 2018

Conversation

davidkyle
Copy link
Member

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 in Job.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

@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.

LGTM

@davidkyle
Copy link
Member Author

retest this please

@davidkyle davidkyle merged commit ef5007b into elastic:master Oct 4, 2018
@davidkyle davidkyle deleted the duplicate-parsing branch October 4, 2018 12:16
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)
  ...
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.

4 participants