Add implementation to update service settings method for Alibaba Cloud Search service by Jan-Kazlouski-elastic · Pull Request #142738 · elastic/elasticsearch

Jan-Kazlouski-elastic · 2026-02-19T22:33:33Z

This PR is a part of a broader initiative related to endpoint _update operation: #122356
In particular - this PR provides implementation for updateServiceSettings method used in _update operation for Alibaba Cloud Search service.
Initially the changes present in this PR were a part of a bigger PR #140082.
API reference for the update operation can be found here: https://www.elastic.co/docs/api/doc/elasticsearch/operation/operation-inference-update.

service_id, host, workspace, similarity and dimensions in service settings map are immutable, meaning they cannot be updated with _update operation.

- Have you signed the contributor license agreement?
- Have you followed the contributor guidelines?
- If submitting code, have you built your formula locally prior to submission with gradle check?
- If submitting code, is your pull request against main? Unless there is a good reason otherwise, we prefer pull requests against main and will backport as needed.
- If submitting code, have you checked that your submission is for an OS and architecture that we support?
- If you are submitting this code for a class then read our policy for that.

…ceSettings and related classes

elasticsearchmachine · 2026-02-19T22:33:57Z

Pinging @elastic/search-inference-team (Team:Search - Inference)

…ice-settings-method-alibabacloudsearch

Jan-Kazlouski-elastic · 2026-02-20T11:08:34Z

...ence/services/alibabacloudsearch/completion/AlibabaCloudSearchCompletionServiceSettings.java

    public static final String NAME = "alibabacloud_search_completion_service_settings";

    public static AlibabaCloudSearchCompletionServiceSettings fromMap(Map<String, Object> map, ConfigurationParseContext context) {
-        ValidationException validationException = new ValidationException();


ValidationException was not passed anywhere and couldn't be filled out with any validation errors. Actual validation was performed only within the AlibabaCloudSearchServiceSettings.fromMap method

Jan-Kazlouski-elastic · 2026-02-20T11:16:43Z

...ence/services/alibabacloudsearch/embeddings/AlibabaCloudSearchEmbeddingsServiceSettings.java

    public static AlibabaCloudSearchEmbeddingsServiceSettings fromMap(Map<String, Object> map, ConfigurationParseContext context) {
-        ValidationException validationException = new ValidationException();
+        var validationException = new ValidationException();
        var commonServiceSettings = AlibabaCloudSearchServiceSettings.fromMap(map, context);


As you may see, the validation is happening in 2 steps here. If AlibabaCloudSearchServiceSettings.fromMap introduces validation fails - it will fail early without checking embedding specific fields and without filling out the potential errors in these fields.
I don't see this approach as optimal. We could alter the signature of the AlibabaCloudSearchServiceSettings.fromMap to take validation exception as param and fill it out with validation errors for the common fields while keeping the responsibility of throwing the exception on task specific classes. I implemented that locally. If it is decided that we would want to go with this approach I can push it here.

CC @DonalEvans

I like the idea of passing the ValidationException into AlibabaCloudSearchServiceSettings.fromMap(). The purpose of the way we use ValidationException is to allow us to capture ALL of the validation problems with a given input, so having two places we can throw while parsing a single object feels wrong.

...ticsearch/xpack/inference/services/alibabacloudsearch/AlibabaCloudSearchServiceSettings.java

DonalEvans

A few suggestions for minor test improvements, but nothing worth holding the PR up for if you don't feel like implementing the changes.

DonalEvans · 2026-02-20T22:26:51Z

...services/alibabacloudsearch/completion/AlibabaCloudSearchCompletionServiceSettingsTests.java

+        );
+    }
+
+    public void testUpdateServiceSettings_EmptyMap_Success() {


Rather than simply *_Success this test might be better named something like *_DoesNotChangeSettings

Done. Also changed it for AI21 so the approach is the same for both services. I want to keep same more or less unified across the services.

DonalEvans · 2026-02-20T22:28:48Z

...services/alibabacloudsearch/completion/AlibabaCloudSearchCompletionServiceSettingsTests.java

+        var serviceSettings = new AlibabaCloudSearchCompletionServiceSettings(
+            new AlibabaCloudSearchServiceSettings(
+                INITIAL_TEST_SERVICE_ID,
+                INITIAL_TEST_HOST,
+                INITIAL_TEST_WORKSPACE_NAME,
+                INITIAL_TEST_HTTP_SCHEMA,
+                new RateLimitSettings(INITIAL_TEST_RATE_LIMIT)
+            )
+        ).updateServiceSettings(new HashMap<>());
+
+        assertThat(
+            serviceSettings,
+            is(
+                new AlibabaCloudSearchCompletionServiceSettings(
+                    new AlibabaCloudSearchServiceSettings(
+                        INITIAL_TEST_SERVICE_ID,
+                        INITIAL_TEST_HOST,
+                        INITIAL_TEST_WORKSPACE_NAME,
+                        INITIAL_TEST_HTTP_SCHEMA,
+                        new RateLimitSettings(INITIAL_TEST_RATE_LIMIT)
+                    )
+                )
+            )
+        );


This test can be simplified/clarified a little if we do:

var originalServiceSettings = new AlibabaCloudSearchCompletionServiceSettings( new AlibabaCloudSearchServiceSettings( INITIAL_TEST_SERVICE_ID, INITIAL_TEST_HOST, INITIAL_TEST_WORKSPACE_NAME, INITIAL_TEST_HTTP_SCHEMA, new RateLimitSettings(INITIAL_TEST_RATE_LIMIT) ) ); var updatedServiceSettings = originalServiceSettings.updateServiceSettings(new HashMap<>()); assertThat(updatedServiceSettings, is(originalServiceSettings));

Good idea. Done that for AI21 as well.

...services/alibabacloudsearch/completion/AlibabaCloudSearchCompletionServiceSettingsTests.java

DonalEvans · 2026-02-20T22:32:47Z

...services/alibabacloudsearch/embeddings/AlibabaCloudSearchEmbeddingsServiceSettingsTests.java

+        );
+    }
+
+    public void testUpdateServiceSettings_EmptyMap_Success() {


See comments on AlibabaCloudSearchCompletionServiceSettingsTests.testUpdateServiceSettings_EmptyMap_Success()

...services/alibabacloudsearch/embeddings/AlibabaCloudSearchEmbeddingsServiceSettingsTests.java

DonalEvans · 2026-02-20T23:20:46Z

...ference/services/alibabacloudsearch/rerank/AlibabaCloudSearchRerankServiceSettingsTests.java

+        );
+    }
+
+    public void testUpdateServiceSettings_EmptyMap_Success() {


See comments on AlibabaCloudSearchCompletionServiceSettingsTests.testUpdateServiceSettings_EmptyMap_Success()

DonalEvans · 2026-02-20T23:24:14Z

...ference/services/alibabacloudsearch/sparse/AlibabaCloudSearchSparseServiceSettingsTests.java

+        );
+    }
+
+    public void testUpdateServiceSettings_EmptyMap_Success() {


See comments on AlibabaCloudSearchCompletionServiceSettingsTests.testUpdateServiceSettings_EmptyMap_Success()

DonalEvans · 2026-02-20T23:25:34Z

...arch/xpack/inference/services/alibabacloudsearch/AlibabaCloudSearchServiceSettingsTests.java

+        );
+    }
+
+    public void testUpdateServiceSettings_EmptyMap_Success() {


See comments on AlibabaCloudSearchCompletionServiceSettingsTests.testUpdateServiceSettings_EmptyMap_Success()

…ice-settings-method-alibabacloudsearch

Jan-Kazlouski-elastic · 2026-02-23T09:13:07Z

Hi @DonalEvans ! Thank you for your comments and approval. I implemented the proposed fixes and a few more. Also unified testing approaches with the one taken for AI21. I want to keep everything more or less the same across the implementations.
Would you please give this PR another look so we could merge this one?

…on-sliced-reindex * upstream/main: (110 commits) Add search task watchdog to log hot threads on slow search (elastic#142746) Fix return_intermediate_results query param on get async search results (elastic#142875) Mute org.elasticsearch.compute.operator.exchange.BatchDriverTests testSinglePageSingleBatch elastic#142895 Cancel reindex body always has status (elastic#142766) Fix built-in roles sync losing updates (elastic#142433) ESQL: Clarify docs and add csv test for WHERE in STATS (elastic#133629) Fix and unmute ReindexResumeIT (elastic#142788) Fix broken release notes Mute org.elasticsearch.benchmark.vector.scorer.VectorScorerOSQBenchmarkTests testSingleScalarVsVectorized {p0=384 p1=4 p2=NIO p3=COSINE} elastic#142883 ES|QL: fix Generative tests for commands that don't change the output schema (elastic#142864) Mute org.elasticsearch.benchmark.vector.scorer.VectorScorerOSQBenchmarkTests testSingleScalarVsVectorized {p0=1024 p1=1 p2=NIO p3=DOT_PRODUCT} elastic#142881 SQL: Fix QlIllegalArgumentException with non-foldable date range queries (elastic#142386) Add more errors to the allowed_errors with github issue links (elastic#142862) ESQL: reapply "NDJSON datasource" (elastic#142855) Add implementation to update service settings method for Alibaba Cloud Search service (elastic#142738) Mute org.elasticsearch.snapshots.SnapshotShutdownIT testStartRemoveNodeButDoNotComplete elastic#142871 Mute org.elasticsearch.snapshots.SnapshotShutdownIT testDeleteSnapshotWithPausedShardSnapshots elastic#142870 Mute org.elasticsearch.snapshots.SnapshotShutdownIT testAbortSnapshotWhileRemovingNode elastic#142869 Mute org.elasticsearch.snapshots.SnapshotShutdownIT testRemoveNodeDuringSnapshot elastic#142868 ES|QL: Guard exponential_histogram TO_STRING against too large inputs (elastic#140718) ...

…d Search service (elastic#142738) * Add updateServiceSettings method to AlibabaCloudSearchCompletionServiceSettings and related classes * Fix unit tests and add changelog * Fix validation exception handling * Refactor updateServiceSettings tests to improve clarity and consistency

Add updateServiceSettings method to AlibabaCloudSearchCompletionServi…

f8a936f

…ceSettings and related classes

Jan-Kazlouski-elastic requested review from DonalEvans, dimitris-athanasiou and jonathan-buttner February 19, 2026 22:33

Jan-Kazlouski-elastic assigned DonalEvans Feb 19, 2026

Jan-Kazlouski-elastic added :SearchOrg/Inference Label for the Search Inference team Team:Search - Inference >tech-debt labels Feb 19, 2026

elasticsearchmachine added v9.4.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Feb 19, 2026

Jan-Kazlouski-elastic added 2 commits February 20, 2026 00:35

Fix unit tests and add changelog

811808c

Merge remote-tracking branch 'origin/main' into implement-update-serv…

63e6195

…ice-settings-method-alibabacloudsearch

Jan-Kazlouski-elastic commented Feb 20, 2026

View reviewed changes

...ticsearch/xpack/inference/services/alibabacloudsearch/AlibabaCloudSearchServiceSettings.java Show resolved Hide resolved

DonalEvans approved these changes Feb 20, 2026

View reviewed changes

Jan-Kazlouski-elastic added 3 commits February 23, 2026 10:06

Fix validation exception handling

1bf97b5

Refactor updateServiceSettings tests to improve clarity and consistency

eb5f90a

Merge remote-tracking branch 'origin/main' into implement-update-serv…

7f4e317

…ice-settings-method-alibabacloudsearch

Jan-Kazlouski-elastic requested a review from DonalEvans February 23, 2026 15:25

DonalEvans approved these changes Feb 23, 2026

View reviewed changes

Jan-Kazlouski-elastic merged commit d5a327a into elastic:main Feb 23, 2026
37 checks passed

Jan-Kazlouski-elastic deleted the implement-update-service-settings-method-alibabacloudsearch branch April 2, 2026 09:05

Conversation

Jan-Kazlouski-elastic commented Feb 19, 2026

Uh oh!

elasticsearchmachine commented Feb 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DonalEvans left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jan-Kazlouski-elastic commented Feb 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants