Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable proxy use and other httpOptions goodness, Extract aws-sdk configuration from SDK if not provided. #8

Closed
wants to merge 3 commits into from

Conversation

erikerikson
Copy link

Replace null with the httpOptions object that aws-sdk is configured with. As per the sdk, handleRequest requires those be passed in. This allows the use of proxies via agent and other httpOptions goodness.

…is allows the use of proxies and other agent goodness.
@jinqianlee
Copy link

Assume the user created a standard awsConfig object, It is desired to have a new field 'httpOptions' in the amazonES object to accept whatever options the user had in the awsConfig object:

amazonES: {
region: awsConfig.region,
credentials: awsConfig.credentials,
httpOptions: awsConfig.httpOptions
}

On line 91, the code can be changed to receive the htttpOptions from amazonES object:

req = send.handleRequest(request, this.amazonES.httpOptions, function (_incoming) {
...
});

…the request.

Make the amazonES configuration parameter optional, allowing for the configuration to be extracted from the existing configuration of aws-sdk.
@erikerikson
Copy link
Author

This is a good point. Consuming code should be able to explicitly provide the httpOptions if desired.

It is also nice to not have to explicitly provide the configuration at all. See last commit.

@erikerikson erikerikson changed the title Enable proxy use and other httpOptions goodness Enable proxy use and other httpOptions goodness, Extract aws-sdk configuration from SDK if not provided. Mar 13, 2016
@danieleorler
Copy link

@erikerikson: I think it might be better to split this PR in two, one for httpOptions and one for aws-sdk configuration. I would be easier to merge I guess.

@TheDeveloper
Copy link
Owner

Thanks for this. I will be making a new release once I have implemented tests for this module.

connector-es6.js Outdated
return AWS.config.credentials;
}
}
getHttpOptions() {

Choose a reason for hiding this comment

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

Should this function definition have amazonEs passed in as a parameter?

Copy link
Author

Choose a reason for hiding this comment

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

Erm... Yes. Yes, it should!

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.

5 participants