Skip to content

Conversation

@dimitris-athanasiou
Copy link
Contributor

We currently have two different native processes:
autodetect & normalizer. There are plans for introducing
a new process. All these share many things in common.
This commit refactors the processes to extend an
AbstractNativeProcess class that encapsulates those
commonalities with the purpose of reusing the code
for new processes in the future.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

Copy link
Member

@davidkyle davidkyle left a comment

Choose a reason for hiding this comment

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

LGTM. This appears to be a standard refactor none of the code has changed it has just be moved around

Copy link
Member

Choose a reason for hiding this comment

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

++ we should change the other uses of the deprecated Loggers.getLogger

Copy link
Member

Choose a reason for hiding this comment

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

nit: trying to avoid our profligate and overloaded use of job perhaps rename to persistState

Copy link
Member

Choose a reason for hiding this comment

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

nit: implementation details in the interface javadoc

@dimitris-athanasiou dimitris-athanasiou force-pushed the extract-common-native-process-base-class branch from 31f92b8 to 62b147b Compare October 25, 2018 15:29
Copy link

@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

@dimitris-athanasiou
Copy link
Contributor Author

jenkins retest this please

We currently have two different native processes:
autodetect & normalizer. There are plans for introducing
a new process. All these share many things in common.
This commit refactors the processes to extend an
`AbstractNativeProcess` class that encapsulates those
commonalities with the purpose of reusing the code
for new processes in the future.
@dimitris-athanasiou dimitris-athanasiou force-pushed the extract-common-native-process-base-class branch from 62b147b to b99ec87 Compare October 26, 2018 11:02
@dimitris-athanasiou dimitris-athanasiou merged commit a39a67c into elastic:master Oct 26, 2018
@dimitris-athanasiou dimitris-athanasiou deleted the extract-common-native-process-base-class branch October 26, 2018 14:34
dimitris-athanasiou added a commit that referenced this pull request Oct 26, 2018
We currently have two different native processes:
autodetect & normalizer. There are plans for introducing
a new process. All these share many things in common.
This commit refactors the processes to extend an
`AbstractNativeProcess` class that encapsulates those
commonalities with the purpose of reusing the code
for new processes in the future.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Oct 26, 2018
* 'master' of github.com:elastic/elasticsearch:
  Fix line length for org.elasticsearch.common.* files (elastic#34888)
  [ML] Extract common native process base class (elastic#34856)
  Refactor children aggregator into a generic ParentJoinAggregator (elastic#34845)
  [Style] Fix line lengths in action.admin.indices (elastic#34890)
  HLRC - add support for source exists API (elastic#34519)
  [CCR] Retry when no index shard stats can be found (elastic#34852)
  [Docs] audit logfile structured format (elastic#34584)
  [Test] Fix FullClusterRestartIT.testShrink() with copy_settings param (elastic#34853)
  Fix LineLength Check Suppressions: index.fielddata (elastic#34891)
  TEST: Stablize Minio Free Port Search (elastic#34894)
  Delete flaky SettingsBasedHostProviderIT test (elastic#34813)
  [ML] Include message in field_stats for text log files (elastic#34861)
  [TEST] HLRC: Expand failure messages in API checks (elastic#34838)
  Lowercase static final DeprecationLogger instance names (elastic#34887)
jasontedor added a commit to martijnvg/elasticsearch that referenced this pull request Oct 26, 2018
* master:
  Introduce cross-cluster replication API docs (elastic#34726)
  Responses can use Writeable.Reader interface (elastic#34655)
  SQL: Provide null-safe scripts for Not and Neg (elastic#34877)
  Fix put/resume follow request parsing (elastic#34913)
  Fix line length for org.elasticsearch.common.* files (elastic#34888)
  [ML] Extract common native process base class (elastic#34856)
  Refactor children aggregator into a generic ParentJoinAggregator (elastic#34845)
  [Style] Fix line lengths in action.admin.indices (elastic#34890)
  HLRC - add support for source exists API (elastic#34519)
kcm pushed a commit that referenced this pull request Oct 30, 2018
We currently have two different native processes:
autodetect & normalizer. There are plans for introducing
a new process. All these share many things in common.
This commit refactors the processes to extend an
`AbstractNativeProcess` class that encapsulates those
commonalities with the purpose of reusing the code
for new processes in the future.
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