Skip to content

[Enhancement] allow using custom Client #158

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

Merged
merged 1 commit into from
Oct 11, 2016
Merged

[Enhancement] allow using custom Client #158

merged 1 commit into from
Oct 11, 2016

Conversation

dmaicher
Copy link
Contributor

@dmaicher dmaicher commented Oct 9, 2016

Issues: #138 & sendgrid/java-http-client#8

Will allow to pass in a custom Client (that itself might have a custom CloseableHttpClient to use proxies for example).

I can provide a signed CLA once I know this PR makes sense as it is 😉

SendGrid sg = new SendGrid(SENDGRID_API_KEY, client);
Request request = new Request();
sg.makeCall(request);
verify(client).api(request);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

instead of using a mock I could also add a getter for the client and make sure its the one we passed into the constructor. Opinions? 😊

Copy link

Choose a reason for hiding this comment

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

it's fine as it is

Choose a reason for hiding this comment

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

I am sending Email with attachments. I want to set ConnectionTimeout values like shown above, But Not able to figure out where to set. I created Client object. I used abovbe 4 lines. But I do no see any API on Client or SendGrid to set timeout values. Can someone please help me with this.

@dmaicher
Copy link
Contributor Author

dmaicher commented Oct 9, 2016

Something seems to be wrong in general with the build? Well the test I added should be passing 😉

@dmaicher
Copy link
Contributor Author

And btw I think this PR should be favoured instead of #119 as the SendGrid class should have no knowledge about the http client implementation which is an internal detail of the Client abstraction.

@thinkingserious
Copy link
Contributor

Thanks for the PR @dmaicher,

It looks good to me. I'll be looking forward to your signed CLA :)

To test locally, please follow steps 3-5 here: https://github.com/sendgrid/sendgrid-ruby/blob/master/CONTRIBUTING.md#initial-setup

With Best Regards,

Elmer

@thinkingserious thinkingserious added type: community enhancement feature request not on Twilio's roadmap status: cla needed labels Oct 10, 2016
@dmaicher
Copy link
Contributor Author

@thinkingserious I emailed the CLA 😉

@ikatkov
Copy link

ikatkov commented Oct 11, 2016

+1 lgtm

@thinkingserious thinkingserious merged commit 4a82ea0 into sendgrid:master Oct 11, 2016
@thinkingserious
Copy link
Contributor

@dmaicher,

Could you please take a moment to fill out this form so that we may send you some swag? Thanks!

@thinkingserious
Copy link
Contributor

@ikatkov I would also like to offer you some swag for your help. Please fill out this form. Thanks!

thinkingserious added a commit that referenced this pull request Oct 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: community enhancement feature request not on Twilio's roadmap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants