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.

Sign up for GitHub

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

Jump to bottom

[Enterprise Search] Index Pipelines Inference History #142802

Merged
merged 6 commits into from
Oct 6, 2022
Merged

[Enterprise Search] Index Pipelines Inference History #142802

merged 6 commits into from
Oct 6, 2022

Conversation

TattdCodeMonkey
Copy link
Contributor

@TattdCodeMonkey TattdCodeMonkey commented Oct 5, 2022 •
edited
Loading

Summary

  • Added an endpoint to query the inference history for a given index
  • Added the Inference history tab to the index pipelines page

image

Checklist

@TattdCodeMonkey TattdCodeMonkey added release_note:skip Skip the PR/issue when compiling release notes Team:EnterpriseSearch v8.6.0 labels Oct 5, 2022
@TattdCodeMonkey TattdCodeMonkey requested a review from a team October 5, 2022 21:28
@TattdCodeMonkey TattdCodeMonkey force-pushed the rnorris/es/ml-pipeline-inference-history branch from e654695 to c6cd791 Compare October 5, 2022 21:36
efegurkan
efegurkan approved these changes Oct 6, 2022
View reviewed changes
Copy link
Member

@efegurkan efegurkan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

We can merge after adding missing path on logic file

...tions/enterprise_search_content/components/search_index/pipelines/inference_history_logic.ts

export const InferenceHistoryLogic = kea<
MakeLogicType<InferenceHistoryValues, InferenceHistoryActions>
>({
Copy link
Member

Choose a reason for hiding this comment

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

path is missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will fix, I need to fix something else too.

@TattdCodeMonkey TattdCodeMonkey force-pushed the rnorris/es/ml-pipeline-inference-history branch from e703ab0 to be63d3a Compare October 6, 2022 14:09
TattdCodeMonkey
TattdCodeMonkey commented Oct 6, 2022
View reviewed changes
x-pack/plugins/enterprise_search/server/routes/enterprise_search/indices.ts
@@ -623,4 +624,26 @@ export function registerIndexRoutes({
}
})
);

router.get(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to add a test for this too, I'll do that today and update this PR.

@TattdCodeMonkey TattdCodeMonkey force-pushed the rnorris/es/ml-pipeline-inference-history branch from be63d3a to dba59db Compare October 6, 2022 16:29
brianmcgue
brianmcgue approved these changes Oct 6, 2022
View reviewed changes
Copy link
Contributor

@brianmcgue brianmcgue left a comment

Choose a reason for hiding this comment

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

Just a little nitpick about spelling. Looks good though!

x-pack/plugins/enterprise_search/server/lib/indices/fetch_ml_inference_pipeline_history.ts Outdated
client: ElasticsearchClient,
index: string
): Promise<MlInferenceHistoryResponse> => {
const ingestPipeleProcessorsResult = await client.search<
Copy link
Contributor

Choose a reason for hiding this comment

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

ingestPipelineProcessorsResult

@TattdCodeMonkey TattdCodeMonkey force-pushed the rnorris/es/ml-pipeline-inference-history branch from dba59db to 0090430 Compare October 6, 2022 22:53
@TattdCodeMonkey TattdCodeMonkey enabled auto-merge (squash) October 6, 2022 22:53
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
enterpriseSearch 1723 1726 +3

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
enterpriseSearch 1.7MB 1.7MB +2.8KB

History

  • 💚 Build #78801 succeeded dba59db5f34c2beaf1c913ade533a16b79895f33
  • 💚 Build #78734 succeeded be63d3a0a9c4f1f45af60e1e790d811ff18d960a
  • 💚 Build #78559 succeeded c6cd7917d6899b5d7241b8caed3b117339cbc071

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@TattdCodeMonkey TattdCodeMonkey merged commit fae5387 into elastic:main Oct 6, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Oct 6, 2022
@sorenlouv
Copy link
Member

Hey @TattdCodeMonkey,

It looks like this PR crossed with @sphilipse's PR (#142881) that introduces some new linting rules for enterprise search. I'm now seeing the following error on main:

/dev/shm/builds/kb-n2-8-spot-f072722c4565512c/elastic/kibana-pull-request/kibana/x-pack/plugins/enterprise_search/server/lib/indices/fetch_ml_inference_pipeline_history.test.ts
--
  | 42:7  error  Variable name `EmptyMockResponse` must match one of the following formats: camelCase, UPPER_CASE    @typescript-eslint/naming-convention
  | 50:7  error  Variable name `ListMockResponse` must match one of the following formats: camelCase, UPPER_CASE     @typescript-eslint/naming-convention
  | 67:7  error  Variable name `ObjectMockResponse` must match one of the following formats: camelCase, UPPER_CASE   @typescript-eslint/naming-convention
  | 85:7  error  Variable name `ExpectedMockResults` must match one of the following formats: camelCase, UPPER_CASE  @typescript-eslint/naming-convention
  |  
  | ✖ 4 problems (4 errors, 0 warnings)

@TattdCodeMonkey
Copy link
Contributor Author

@sqren #142951

WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 11, 2022
@TattdCodeMonkey @WafaaNasr
[Enterprise Search] Index Pipelines Inference History (elastic#142802)
32a8851 
* [Enterprise Search] add inference history endpoint

* [Enterprise Search] show index pipeline inference history

* use snake_case for response keys

* fix: add paths to pipelines logics

* test: inference history endpoint

* refactor: typo in variable name
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 14, 2022
@TattdCodeMonkey @WafaaNasr
[Enterprise Search] Index Pipelines Inference History (elastic#142802)
77bbe47 
* [Enterprise Search] add inference history endpoint

* [Enterprise Search] show index pipeline inference history

* use snake_case for response keys

* fix: add paths to pipelines logics

* test: inference history endpoint

* refactor: typo in variable name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brianmcgue brianmcgue brianmcgue approved these changes

@efegurkan efegurkan efegurkan approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:EnterpriseSearch v8.6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@TattdCodeMonkey @kibana-ci @sorenlouv @efegurkan @brianmcgue @kibanamachine