-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Added the typesafe config to read default config values #645
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
Conversation
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
I'm concerned by AbstractAsyncHttpClientFactoryTest: tests are not threadsafe as they depend on conflicting System property changes, so random failures might happen.
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?
Hello can this pull request me merged in. I have fixed all the code review comments as requested by the reviewer.
Any ETA as to by when will this pull requests be merged in?
Could you please rebase your commits so this PR can be merged, and squash into a single commit, please?
...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
ae1ace2 to
b175f06
Compare
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.
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."
Added the typesafe config to read default config values
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.
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