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

[META] IngestService needs xContentRegistry for parametized models. #16865

Open
brianf-aws opened this issue Dec 17, 2024 · 2 comments
Open

[META] IngestService needs xContentRegistry for parametized models. #16865

brianf-aws opened this issue Dec 17, 2024 · 2 comments
Labels
ingest-pipeline Meta Meta issue, not directly linked to a PR untriaged

Comments

@brianf-aws
Copy link

Please describe the end goal of this project

Currently search pipelines is able to pass xContentRegistry

final SearchPipelineService searchPipelineService = new SearchPipelineService(
clusterService,
threadPool,
this.environment,
scriptService,
analysisModule.getAnalysisRegistry(),
xContentRegistry,
namedWriteableRegistry,
pluginsService.filterPlugins(SearchPipelinePlugin.class),
client

This allows the search response/request processors to handle customized parametrized local models.

However Ingest processors have no way of having this field meaning you can not ingest documents using a model that requires parametrized arguments such as local asymmetric embedding model . The fix would be to have a contentRegistry within the ingestService class

final IngestService ingestService = new IngestService(
clusterService,
threadPool,
this.environment,
scriptService,
analysisModule.getAnalysisRegistry(),
pluginsService.filterPlugins(IngestPlugin.class),
client,
indicesService
);

as well as configuring the Ingest processor Parameters class to have a xcontentRegistry just how the search response/request processors have the field.

Supporting References

Taken from my comment

opensearch-project/ml-commons#3276 (comment)

Action Items in this change

opensearch-project/ml-commons#3276 (comment)

Issues

Related component

Plugins

@sandeshkr419
Copy link
Contributor

@gaobinlong Hey, what do you think of this?

@gaobinlong
Copy link
Collaborator

@gaobinlong Hey, what do you think of this?

After checking the original issue in ml-commons plugin and the code, I think it makes sense that we add a construction parameter xContentRegistry to IngestService.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ingest-pipeline Meta Meta issue, not directly linked to a PR untriaged
Projects
Status: New
Development

No branches or pull requests

3 participants