Skip to content

Conversation

@nielsbauman
Copy link
Contributor

We originally defined the projectClient method on ProjectResolver as a convenience method to execute API calls for specific projects. That method requires a reference to both a ProjectResolver and a Client.

We now introduce the same method directly on the Client interface and inject a ProjectResolver there, removing the need for a ProjectResolver reference in places that just want to execute API requests on a specific project.

To reduce the number of changes, this change solely focuses on introducing the new method. Future changes will migrate the uses of the original method to the new one and remove the original altogether.

We originally defined the `projectClient` method on `ProjectResolver` as
a convenience method to execute API calls for specific projects. That
method requires a reference to both a `ProjectResolver` and a `Client`.

We now introduce the same method directly on the `Client` interface and
inject a `ProjectResolver` there, removing the need for a
`ProjectResolver` reference in places that just want to execute API
requests on a specific project.

To reduce the number of changes, this change solely focuses on
introducing the new method. Future changes will migrate the uses of the
original method to the new one and remove the original altogether.
@nielsbauman nielsbauman requested a review from a team June 10, 2025 08:04
@nielsbauman nielsbauman requested a review from a team as a code owner June 10, 2025 08:04
@nielsbauman nielsbauman added >non-issue :Distributed Coordination/Network Http and internode communication implementations labels Jun 10, 2025
@elasticsearchmachine elasticsearchmachine added v9.1.0 Team:Distributed Coordination Meta label for Distributed Coordination team labels Jun 10, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

samxbr
samxbr previously approved these changes Jun 11, 2025
Copy link
Contributor

@samxbr samxbr left a comment

Choose a reason for hiding this comment

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

Thanks for making this change Niels! This will make my change for adding project id to PersistentTasksService easier too.

LGTM based on the discussion in the other thread, just post a question.

Copy link
Contributor

@samxbr samxbr left a comment

Choose a reason for hiding this comment

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

Oh I just saw the serverless tests failed, code compilation failed, looks like you need to update the serverless package as well

@samxbr samxbr dismissed their stale review June 11, 2025 08:36

Need to update serverless

Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

Some minor comments plus pending fix for the serverless CI.

Comment on lines 426 to 435
return new FilterClient(this) {
@Override
protected <Request extends ActionRequest, Response extends ActionResponse> void doExecute(
ActionType<Response> action,
Request request,
ActionListener<Response> listener
) {
projectResolver.executeOnProject(projectId, () -> super.doExecute(action, request, listener));
}
};
Copy link
Member

Choose a reason for hiding this comment

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

I suggest we also override the projectClient method here to throw an exception says something like "not supporting nested project client creation". The execution should fail anyway without it due to double setting project-id. But an explicit exception makes it quicker to happen and easier to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea!

@elasticsearchmachine elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Jun 12, 2025
@nielsbauman nielsbauman merged commit 87c6fa7 into elastic:main Jun 12, 2025
19 checks passed
@nielsbauman nielsbauman deleted the project-client-changes branch June 12, 2025 18:19
nielsbauman added a commit that referenced this pull request Jun 12, 2025
We added a new `projectClient` method on `Client` in #129174. We now
update the usages of the old method (on `ProjectResolver`) to use the
new one and we delete the old method.
nielsbauman added a commit that referenced this pull request Jun 12, 2025
We added a new `projectClient` method on `Client` in #129174. We now
update the usages of the old method (on `ProjectResolver`) to use the
new one and we delete the old method.
nielsbauman added a commit that referenced this pull request Jun 13, 2025
We added a new `projectClient` method on `Client` in #129174. We now
update the usages of the old method (on `ProjectResolver`) to use the
new one and we delete the old method.
nielsbauman added a commit to nielsbauman/elasticsearch that referenced this pull request Jun 13, 2025
We added a new `projectClient` method on `Client` in elastic#129174. We now
update the usages of the old method (on `ProjectResolver`) to use the
new one and we delete the old method.
nielsbauman added a commit to nielsbauman/elasticsearch that referenced this pull request Jun 13, 2025
We added a new `projectClient` method on `Client` in elastic#129174. We now
update the usages of the old method (on `ProjectResolver`) to use the
new one and we delete the old method.
nielsbauman added a commit that referenced this pull request Jun 17, 2025
We added a new `projectClient` method on `Client` in #129174. We now
update the usages of the old method (on `ProjectResolver`) to use the
new one and we delete the old method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Coordination/Network Http and internode communication implementations >non-issue serverless-linked Added by automation, don't add manually Team:Distributed Coordination Meta label for Distributed Coordination team v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants