Skip to content

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 😉

Client client = mock(Client.class);
SendGrid sg = new SendGrid(SENDGRID_API_KEY, client);
Request request = new Request();
sg.makeCall(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