[Inference Timeout] Supply inference context to all third party services #131251
Conversation
Samiul-TheSoccerFan
added
>refactoring
:SearchOrg/Relevance
elasticsearchmachine
added
the
Team:SearchOrg
|
Pinging @elastic/search-eng (Team:SearchOrg) |
|
Pinging @elastic/search-relevance (Team:Search - Relevance) |
|
@davidkyle FYI, we are adding support for a configurable |
| super(factory, serviceComponents, context); | ||
| } | ||
|
|
||
| // for testing |
There was a problem hiding this comment.
Nitpick: I don't think we necessarily need this comment in all classes inheriting this constructor.
There was a problem hiding this comment.
I added these comments to stay consistent with the existing approach in classes like BaseElasticsearchInternalService and ElasticsearchInternalService
Just to clarify, are you suggesting we remove these comments entirely from all the classes, or only from constructors that are simply forwarding to super without additional logic?
There was a problem hiding this comment.
If you want to keep them in, that's OK - it was a nitpick, but I thought they were potentially unnecessary. Non blocking comment though. 🙂
| CheckedSupplier<Map<String, SettingsConfiguration>, RuntimeException> configurationMap, | ||
| ClusterService clusterService | ||
| ) { | ||
| this.modelBuilder = modelBuilder; |
There was a problem hiding this comment.
Could you call this(.... context.clusterService()) in the other constructor instead of duplicating initialization logic?
|
@elasticmachine update branch |
...plugin/inference/src/main/java/org/elasticsearch/xpack/inference/services/SenderService.java
Outdated
Show resolved
Hide resolved
...main/java/org/elasticsearch/xpack/inference/services/amazonbedrock/AmazonBedrockService.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/elasticsearch/xpack/inference/services/elastic/ElasticInferenceService.java
Show resolved
Hide resolved
...nce/src/main/java/org/elasticsearch/xpack/inference/services/sagemaker/SageMakerService.java
Outdated
Show resolved
Hide resolved
|
@elasticmachine update branch |
This PR is part - 1 of the implementation of Inference Timeout. The original PR is getting too big so we planned to tackle some refactoring work in this PR.
This PR includes:
inference factory contextto all third party inference services.