Skip to content

Pass Transport object as the context when callWithRequest is asked to use a transport method#6027

Merged
spalger merged 1 commit intoelastic:masterfrom
Bargs:fix/callWithRequestTransport
Feb 1, 2016
Merged

Pass Transport object as the context when callWithRequest is asked to use a transport method#6027
spalger merged 1 commit intoelastic:masterfrom
Bargs:fix/callWithRequestTransport

Conversation

@Bargs
Copy link
Copy Markdown
Contributor

@Bargs Bargs commented Jan 28, 2016

callWithRequest assumes every elasticsearch.js API method needs the client object as its context, but this isn't the case. Transport.request expects the transport object itself as context.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd feel a lot better about this if it used var toPath = require('lodash/internal/toPath'); to parse the path and then did something like this:

const path = toPath(endpoint);
const api = get(client, path);
const apiContext = get(client, path.slice(0, -1), client);
return api.call(apiContext, ...

… when called normally instead of assuming client is the context for everything
@Bargs Bargs force-pushed the fix/callWithRequestTransport branch from 21c4660 to 580ff60 Compare January 28, 2016 16:22
@Bargs
Copy link
Copy Markdown
Contributor Author

Bargs commented Jan 28, 2016

@spalger updated the commit based on your advice. I had to add a separate isEmpty check instead of providing client as a default since slice returns an empty array, not undefined when it's range doesn't include any elements. Please take another quick look!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure why you can't use _.get()'s third argument here.

_.get({ fakeClient: true }, []) // returns undefined
_.get({ fakeClient: true }, [], 'pasta') // returns 'pasta'
_.get(client, path.slice(0, -1), client); // returns client when path is [], or doesn't resolve to anything for some reason

@spalger spalger added the review label Feb 1, 2016
spalger added a commit that referenced this pull request Feb 1, 2016
Pass Transport object as the context when callWithRequest is asked to use a transport method
@spalger spalger merged commit 2c75de1 into elastic:master Feb 1, 2016
spalger added a commit that referenced this pull request Mar 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants