Skip to content

[ML-Dataframe] Use AsyncTwoPhaseIndexer#33504

Merged
hendrikmuhs merged 4 commits intoelastic:feature/fibfrom
hendrikmuhs:feature/fib-indexer
Sep 10, 2018
Merged

[ML-Dataframe] Use AsyncTwoPhaseIndexer#33504
hendrikmuhs merged 4 commits intoelastic:feature/fibfrom
hendrikmuhs:feature/fib-indexer

Conversation

@hendrikmuhs
Copy link
Copy Markdown

@hendrikmuhs hendrikmuhs commented Sep 7, 2018

FEATURE BRANCH PR

Replace mocked indexer and use AsyncTwoPhaseIndexer (introduced in #32743) instead.

Note: Inner operations are still mocked, although the destination index now uses the job id as a suffix.

@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/ml-core

Copy link
Copy Markdown

@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

I just left one suggestion about an exception type.

bulkIndexRequest.add(new IndexRequest(PIVOT_INDEX, DOC_TYPE).source(builder));

} catch (IOException e) {
throw new RuntimeException(e);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There is a class called UncheckedIOException that might be better here, as it's clearer to the handling code that it should be unwrapped.

@hendrikmuhs hendrikmuhs merged commit 4946d33 into elastic:feature/fib Sep 10, 2018
@hendrikmuhs hendrikmuhs deleted the feature/fib-indexer branch October 18, 2018 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>feature :ml Machine learning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants