-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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: Add ML Get Records API #33085
HLRC: Add ML Get Records API #33085
Conversation
Pinging @elastic/ml-core |
Pinging @elastic/es-core-infra |
long majorAndCriticalCount = recordStats.majorCount + recordStats.criticalCount; | ||
assertThat(response.count(), equalTo(majorAndCriticalCount)); | ||
assertThat(response.records().size(), equalTo((int) Math.min(100, majorAndCriticalCount))); | ||
assertThat(response.records().stream().anyMatch(b -> b.getRecordScore() < 50.0), is(false)); |
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.
So, this could be simplified to assertFalse
Or could be something like the following (which admittedly, is probably less simple)
import static org.hamcrest.CoreMatchers.everyItem;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.beans.HasPropertyWithValue.hasProperty;
...
assertThat(response.records(), everyItem(hasProperty("recordScore", greaterThanOrEqualTo(50))));
Man, that is frustrating that hamcrest does not support just passing a lambda as a type of matcher :(
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.
I try to avoid assertFalse
as on failure the error message is not very helpful (it simply says assertion failed
instead of giving some feedback on the type of assertion). I would prefer leaving as is.
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.
Fine with me, just a thought :)
8f0d0be
to
df0111d
Compare
* 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)
* 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)
Relates #29827