You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "ankitsultana (via GitHub)" <gi...@apache.org> on 2024/02/23 07:12:14 UTC
[PR] Propagate Disable User Agent Config to Http Client [pinot]
ankitsultana opened a new pull request, #12479:
URL: https://github.com/apache/pinot/pull/12479
In #10895 I introduced this config, but I had made an error. I never actually set this config in the final Apache HttpClient that gets created. This fixes that.
**Test Plan**: Several integration/unit-tests that run as part of the PR use the HttpClient so that should take care of smoke testing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Propagate Disable User Agent Config to Http Client [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on PR #12479:
URL: https://github.com/apache/pinot/pull/12479#issuecomment-1963500683
I am not sure how to test this via UTs. The user-agent config is passed to the Apache Commons HttpClient.
After the client is created, there's no way to verify what the user-agent config value is. You can refer to `org.apache.http.impl.client.HttpClientBuilder#build`. The part where this config is used is in the screenshot in the description.
Existing UTs take care of sanity testing the change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Propagate Disable User Agent Config to Http Client [pinot]
Posted by "satishd (via GitHub)" <gi...@apache.org>.
satishd commented on PR #12479:
URL: https://github.com/apache/pinot/pull/12479#issuecomment-1970493679
Created a followup issue https://github.com/apache/pinot/issues/12519
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Propagate Disable User Agent Config to Http Client [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #12479:
URL: https://github.com/apache/pinot/pull/12479
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Propagate Disable User Agent Config to Http Client [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12479:
URL: https://github.com/apache/pinot/pull/12479#issuecomment-1960869640
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12479?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `0%` with `2 lines` in your changes are missing coverage. Please review.
> Project coverage is 0.00%. Comparing base [(`6b0cfeb`)](https://app.codecov.io/gh/apache/pinot/commit/6b0cfebffe6a01a1f7eb28dbbc2ac3c0e8e297c6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`56d8697`)](https://app.codecov.io/gh/apache/pinot/pull/12479?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 1 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/12479?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...org/apache/pinot/common/utils/http/HttpClient.java](https://app.codecov.io/gh/apache/pinot/pull/12479?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvaHR0cC9IdHRwQ2xpZW50LmphdmE=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12479?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12479 +/- ##
=============================================
- Coverage 46.87% 0.00% -46.88%
=============================================
Files 1829 2360 +531
Lines 96638 129472 +32834
Branches 15656 20078 +4422
=============================================
- Hits 45300 0 -45300
- Misses 48104 129472 +81368
+ Partials 3234 0 -3234
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/12479/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration](https://app.codecov.io/gh/apache/pinot/pull/12479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (?)` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/12479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (?)` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/12479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/12479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-46.72%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-46.87%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/12479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-46.66%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/12479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-46.88%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/12479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12479/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/12479?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Propagate Disable User Agent Config to Http Client [pinot]
Posted by "satishd (via GitHub)" <gi...@apache.org>.
satishd commented on PR #12479:
URL: https://github.com/apache/pinot/pull/12479#issuecomment-1964320158
We can extract a method like below and add a UT for this method to check whether the `HttpClientBuilder` instance is updated with all the targeted configs passed to `HttpClientConfig` while building `HttpClient`. WDYT?
```
static void setConfigsToHttpClientBuilder(HttpClientConfig httpClientConfig, HttpClientBuilder httpClientBuilder) {
if (httpClientConfig.getMaxConnTotal() > 0) {
httpClientBuilder.setMaxConnTotal(httpClientConfig.getMaxConnTotal());
}
if (httpClientConfig.getMaxConnPerRoute() > 0) {
httpClientBuilder.setMaxConnPerRoute(httpClientConfig.getMaxConnPerRoute());
}
if (httpClientConfig.isDisableDefaultUserAgent()) {
httpClientBuilder.disableDefaultUserAgent();
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Propagate Disable User Agent Config to Http Client [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #12479:
URL: https://github.com/apache/pinot/pull/12479#issuecomment-1970184623
Since the fix is very straight forward, I'll merge it now. We can consider adding a test separately
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org