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

HLRC: Move ML protocol classes into client ml package #33203

Conversation

dimitris-athanasiou
Copy link
Contributor

No description provided.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@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

Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

Green means GO! LTGM as long the build is green.

@dimitris-athanasiou dimitris-athanasiou force-pushed the move-ml-protocol-classes-into-client-ml-package branch from 8f2b0c8 to 39fff7c Compare August 28, 2018 14:33
Copy link
Contributor

@hub-cap hub-cap left a comment

Choose a reason for hiding this comment

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

one Q. Ill let you decide and not block the merge. I assume tho there will be more calls to createParser in the future, so we might want to use a common deprecation handler.

@@ -59,6 +59,15 @@
};
}

private static final DeprecationHandler DEPRECATION_HANDLER = new DeprecationHandler() {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this the only use of DeprecationHandler in the code? There is an instance of this handler in the RestHighLevelClient as well, so it might make more sense to use the one there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is actually the only use. Though we could reuse the one in RestHighLevelClient, it is an odd dependency direction. I am tempted to duplicate for now. If we hit another we can refactor.

@benwtrent benwtrent merged commit 5697d93 into elastic:master Aug 28, 2018
benwtrent pushed a commit that referenced this pull request Aug 28, 2018
* HLRC: Move ML protocol classes into client ml package

* Do not use log4j deprecation handler

* JavaDoc should refer to correct Job path
@dimitris-athanasiou dimitris-athanasiou deleted the move-ml-protocol-classes-into-client-ml-package branch August 28, 2018 20:51
dnhatn added a commit that referenced this pull request Aug 29, 2018
* master:
  Painless: Add Bindings (#33042)
  Update version after client credentials backport
  Fix forbidden apis on FIPS (#33202)
  Remote 6.x transport BWC Layer for `_shrink` (#33236)
  Test fix - Graph HLRC tests needed another field adding to randomisation exception list
  HLRC: Add ML Get Records API (#33085)
  [ML] Fix character set finder bug with unencodable charsets (#33234)
  TESTS: Fix overly long lines (#33240)
  Test fix - Graph HLRC test was missing field name to be excluded from randomisation logic
  Remove unsupported group_shard_failures parameter (#33208)
  Update BucketUtils#suggestShardSideQueueSize signature (#33210)
  Parse PEM Key files leniantly (#33173)
  INGEST: Add Pipeline Processor (#32473)
  Core: Add java time xcontent serializers (#33120)
  Consider multi release jars when running third party audit (#33206)
  Update MSI documentation (#31950)
  HLRC: create base timed request class (#33216)
  [DOCS] Fixes command page titles
  HLRC: Move ML protocol classes into client ml package (#33203)
  Scroll queries asking for rescore are considered invalid (#32918)
  Painless: Fix Semicolon Regression (#33212)
  ingest: minor - update test to include dissect (#33211)
  Switch remaining LLREST usage to new style Requests (#33171)
  HLREST: add reindex API (#32679)
dnhatn added a commit that referenced this pull request Aug 29, 2018
* 6.x:
  Fix forbidden apis on FIPS (#33202)
  HLRC: Add ML Get Records API (#33085)
  [ML] Fix character set finder bug with unencodable charsets (#33234)
  Tests fix - Graph HLRC client overly long line and syncing core’s copy of GraphExploreResponseTests taken from protocol. Related to #33231
  Test fix - Graph HLRC test was missing field name to be excluded from randomisation logic
  Parse PEM Key files leniantly (#33173)
  Core: Add java time xcontent serializers (#33120)
  Consider multi release jars when running third party audit (#33206)
  Update MSI documentation (#31950)
  [DOCS] Fixes command page titles
  HLRC: Move ML protocol classes into client ml package (#33203)
  Painless: Fix Semicolon Regression (#33212)
@benwtrent benwtrent removed the :ml Machine learning label Oct 25, 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.

6 participants