Skip to content

Conversation

Mpdreamz
Copy link
Member

@Mpdreamz Mpdreamz commented Apr 10, 2019

Note: can not be merged while we want to release an alpha for 7,x

Before this PR how the highlevel client performed a call was roughly

client.Search => Dispatch => LowLevelDispatch.SearchDispatch => lowlevelClient.Search => lowlevelClient.DoRequest

The benefit here is that the high level client uses the actual low level client overloads which in turn are responsible for building the url paths. This is all tested through our extensive integration tests.

This PR cuts out the middle layer:

client.Search => lowlevelClient.DoRequest

This means that in the high level client IRequest now does it own url building. Check out ApiUrls LookupUrl in this PR for the first attempt this PR introduces to do this somewhat efficiently.

The upside is that the call chain is much much lower and we calling the client API methods allocate a whole lot less lambda closures.

Downside is that we have two url build mechanisms one in the high level client which is heavily tested but leaves the lowlevel client somehwat exposes. Good thing we will be reintroducing the YAML tests for the low level client again 😄

The hand off from RouteValues to LookupUrl is less clean than I had liked.

IndexRequest needs a resolved id to determine whether to do a PUT or POST. This warrants more investigation but would like to do so as part of a new branch as this one is rather large already.

@Mpdreamz Mpdreamz mentioned this pull request Apr 11, 2019
Copy link
Contributor

@russcam russcam left a comment

Choose a reason for hiding this comment

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

LGTM 👍

russcam added 2 commits April 12, 2019 17:39
# Conflicts:
#	src/Nest/CommonAbstractions/ConnectionSettings/ConnectionSettingsBase.cs
#	src/Nest/_Generated/_Descriptors.generated.cs
#	src/Nest/_Generated/_LowLevelDispatch.generated.cs
#	src/Nest/_Generated/_Requests.generated.cs
@russcam russcam merged commit 8a5a2fe into 7.x Apr 12, 2019
@Mpdreamz Mpdreamz deleted the refactor/remove-dispatch branch June 17, 2019 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants