Skip to content

Conversation

@salilsurendran
Copy link
Contributor

Added the library typesafe config to read default config values. The order followed is system properties, asynchttpclient.properties and then defaultahc.properties. defaultahc.properties has all the default values for each of the configuration properties and these could either be overridden by a system property or asynchttpclient.properties

@salilsurendran
Copy link
Contributor Author

At the point I made this pull request there are 4 tests not related to my code changes that are failing:
Failed tests:
GrizzlyAsyncStreamHandlerTest>AsyncStreamHandlerTest.asyncStream302WithBody:361 expected [302] but found [301]
GrizzlyRemoteSiteTest>RemoteSiteTest.testGoogleComWithTimeout:122 expected [302] but found [301]
Failed tests:
NettyAsyncStreamHandlerTest>AsyncStreamHandlerTest.asyncStream302WithBody:361 expected [302] but found [301]
NettyRemoteSiteTest>RemoteSiteTest.testGoogleComWithTimeout:122 expected [302] but found [301]

These failures are preventing the tests in extra from running. However, I have run those tests individually to make sure that nothing is broken

@slandelle
Copy link
Contributor

I removed AsyncStreamHandlerTest.asyncStream302WithBody that didn't make sense and "fixed" RemoteSiteTest.testGoogleComWithTimeout. In both cases, the problem was that Google redirects differently depending on geoloc.

Copy link
Contributor

Choose a reason for hiding this comment

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

typo

@slandelle
Copy link
Contributor

I'm concerned by AbstractAsyncHttpClientFactoryTest: tests are not threadsafe as they depend on conflicting System property changes, so random failures might happen.

@salilsurendran
Copy link
Contributor Author

AbstractAsyncHttpClientFactoryTest has been there for quite some time and we have been running these tests in our production CI without any failures, but we are not running testng with method level multithreading but with class level multithreading. Is the AsyncHttpClient CI run with method level multithreading?

@salilsurendran
Copy link
Contributor Author

Hello can this pull request me merged in. I have fixed all the code review comments as requested by the reviewer.

@salilsurendran
Copy link
Contributor Author

Any ETA as to by when will this pull requests be merged in?

@slandelle
Copy link
Contributor

Hi @salilsurendran

Could you please rebase your commits so this PR can be merged, and squash into a single commit, please?

@slandelle slandelle added this to the 2.0.0.Alpha1 milestone Sep 4, 2014
@slandelle slandelle self-assigned this Sep 4, 2014
…order followed is system properties, asynchttpclient.properties and then defaultahc.properties. defaultahc.properties has all the default values for each of the configuration properties and these could either be overridden by a system property or asynchttpclient.properties
@salilsurendran
Copy link
Contributor Author

Hello Stephane,
I squashed all my commits into one. I ran the unit tests on the main branch without my changes and saw that 10 unit tests were failing. On my branch there are no extra unit test failures and the unit tests I added are successful. So please do merge my pull request.
Thanks.

@salilsurendran
Copy link
Contributor Author

Hello Stephane,
I squashed all my commits into one. I ran the unit tests on the main
branch without my changes and saw that 10 unit tests were failing. On my
branch there are no extra unit test failures and the unit tests I added are
successful. So please do merge my pull request. Thanks.

On Thu, Sep 4, 2014 at 12:50 AM, Stephane Landelle <notifications@github.com

wrote:

Hi @salilsurendran https://github.com/salilsurendran

Could you please rebase your commits so this PR can be merged, and squash
into a single commit, please?


Reply to this email directly or view it on GitHub
#645 (comment)
.

Thanks,
Salil
"The surest sign that intelligent life exists elsewhere in the universe is
that none of it has tried to contact us."

slandelle pushed a commit that referenced this pull request Sep 8, 2014
Added the typesafe config to read default config values
@slandelle slandelle merged commit d78d481 into AsyncHttpClient:master Sep 8, 2014
cs-workco pushed a commit to cs-workco/async-http-client that referenced this pull request Apr 13, 2023
AsyncHTTPClient now requires at least Swift 5.5.2. The async/await APIs are therefore available in all supported Swift compiler versions.

Swift Concurrency and HTTP/2 are now also supported for some time now. No need to call out the specific version in the readme. This information can still be found in the release notes if needed.
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.

2 participants