search

brandonseydel / MailChimp.Net

Mail Chimp 3.0 Wrapper
MIT License
322 stars 179 forks source link

Convert to one httpclient per resource #323

Closed brandonseydel closed 3 years ago

brandonseydel commented 6 years ago

Should increase throughput of the API.

janhansen commented 5 years ago

By resource do you then mean the entire application?

Anyway! This article explains why HttpClient should be a Singleton: https://aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/

I realize this will mean touching all Logic-classes, removing using-constructs but I guess it will be worth it.

brandonseydel commented 5 years ago

Yea that’s why I made the help request awhile ago.

From: janhansen notifications@github.com Sent: Tuesday, December 4, 2018 12:21 AM To: brandonseydel/MailChimp.Net Cc: Brandon seydel; Author Subject: Re: [brandonseydel/MailChimp.Net] Convert to one httpclient per resource (#323)

By resource do you then mean the entire application?

Anyway! This article explains why HttpClient should be a Singleton: https://aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Faspnetmonsters.com%2F2016%2F08%2F2016-08-27-httpclientwrong%2F&data=02%7C01%7C%7Cab6e2534d3a74be417c308d659b0ab71%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636795012648900506&sdata=Dhen94pA7Yo%2Bv32ByuOg8NLNR77u08dKtdjNCLudBm4%3D&reserved=0

I realize this will mean touching all Logic-classes, removing using-constructs but I guess it will be worth it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fbrandonseydel%2FMailChimp.Net%2Fissues%2F323%23issuecomment-443985379&data=02%7C01%7C%7Cab6e2534d3a74be417c308d659b0ab71%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636795012648900506&sdata=cs9XSn0UXMcaCjcSAqrJy9KHrDFCdtMtF2kSiME6wdY%3D&reserved=0, or mute the threadhttps://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAEzOcYIshq8yKBXbRvAiICMfkELJl_7Uks5u1hRQgaJpZM4V1428&data=02%7C01%7C%7Cab6e2534d3a74be417c308d659b0ab71%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636795012648900506&sdata=YiBIGxZKqTq6Mn8OuvRXRWmCGbseS0mOIe8nROq13oU%3D&reserved=0.

DavidRouyer commented 5 years ago

Libraries like https://github.com/octokit/octokit.net supports HttpClient injection, so you can also use the new HttpClientFactory.

janhansen commented 5 years ago

Would it be acceptable to just implement a wrapper for HttpClient and return that from the call to CreateMailClient? Then the baseaddress could be saved in this wrapper and calls to GetAsync, PostAsync etc. would just call through to the Singleton HttpClient. It would make the change much smaller than retouching all logic-files...

kieron-mcintyre commented 5 years ago

I agree with @janhansen, there doesn't need to be any injection and the instance should not be disposed of after use. A singleton would be appropriate IMO. But this does need doing and throughput should be the least of the concerns (but that would be nice!) when considering it could easily be exhausting connection pools after a batch of requests.