Skip to content

Conversation

benwtrent
Copy link
Member

adds inference request serialization tests that include nlp config updates.

relates: #85863

@benwtrent benwtrent added >test Issues or PRs that are addressing/adding tests :ml Machine learning v8.3.0 labels Apr 13, 2022
@elasticmachine elasticmachine added the Team:ML Meta label for the ML team label Apr 13, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

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

return instance;
InferenceConfigUpdate adjustedUpdate;
InferenceConfigUpdate currentUpdate = instance.getUpdate();
if (currentUpdate instanceof NlpConfigUpdate nlpConfigUpdate) {
Copy link
Member

Choose a reason for hiding this comment

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

Could you use a fancy instanceof switch statement here?

switch (currentUpdate) {
        case NlpConfigUpdate i -> NlpConfigUpdateTests.mutateForVersion(update, version);
        ....
};

Copy link
Member Author

Choose a reason for hiding this comment

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

oh snap, you are correct!

Copy link
Member Author

Choose a reason for hiding this comment

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

Welp, I suppose not. It needs to be added via a compilation flag and it isn't...

@benwtrent
Copy link
Member Author

@elasticmachine update branch

@benwtrent benwtrent merged commit ebc5eca into elastic:master Apr 13, 2022
@benwtrent benwtrent deleted the feature/ml-add-nlp-inference-update-tests branch April 13, 2022 14:32
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Apr 13, 2022
…n/elasticsearch into datastream-reuse-pipeline-source

* 'datastream-reuse-pipeline-source' of github.com:weizijun/elasticsearch: (28 commits)
  Add JDK 19 to Java testing matrix
  [ML] add nlp config update serialization tests (elastic#85867)
  [ML] A text categorization aggregation that works like ML categorization (elastic#80867)
  [ML] Fix serialisation of text embedding updates (elastic#85863)
  TSDB: fix wrong initial value of tsidOrd in TimeSeriesIndexSearcher (elastic#85713)
  Enforce external id uniqueness during DesiredNode construction (elastic#84227)
  Fix Intellij integration (elastic#85866)
  Upgrade Azure SDK to version 12.14.4 (elastic#83884)
  [discovery-gce] Fix initialisation of transport in FIPS mode (elastic#85817)
  Remove unnecessary docs/changelog/85534.yaml
  Prevent ThreadContext header leak when sending response (elastic#68649)
  Add support for impact_areas to health impacts  (elastic#85830)
  Reduce port range re-use in tests (elastic#85777)
  Fix TranslogTests#testStats (elastic#85828)
  Remove hppc from cat allocation api (elastic#85842)
  Fix BuildTests serialization (elastic#85827)
  Use urgent priority for node shutdown cluster state update (elastic#85838)
  Remove Task classes from HLRC (elastic#85835)
  Remove unused migration classes (elastic#85834)
  Remove uses of Charset name parsing (elastic#85795)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ml Machine learning Team:ML Meta label for the ML team >test Issues or PRs that are addressing/adding tests v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants