You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "shrutisinghania (via GitHub)" <gi...@apache.org> on 2025/09/24 11:33:39 UTC
[PR] Add UserAgent in GCS requests [pinot]
shrutisinghania opened a new pull request, #16884:
URL: https://github.com/apache/pinot/pull/16884
This PR addresses the enhancement proposed in issue #16061 by setting a default User-Agent for all Google Cloud Storage (GCS) requests made through the pinot-gcs plugin.
--
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] Add UserAgent in GCS requests [pinot]
Posted by "shrutisinghania (via GitHub)" <gi...@apache.org>.
shrutisinghania commented on PR #16884:
URL: https://github.com/apache/pinot/pull/16884#issuecomment-3329046640
Thank you for the approval, @xiangfu0! Could you please merge this PR?
--
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] Add UserAgent in GCS requests [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #16884:
URL: https://github.com/apache/pinot/pull/16884
--
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] Add UserAgent in GCS requests [pinot]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #16884:
URL: https://github.com/apache/pinot/pull/16884#discussion_r2375948385
##########
pinot-plugins/pinot-file-system/pinot-gcs/src/main/java/org/apache/pinot/plugin/filesystem/GcsPinotFS.java:
##########
@@ -70,9 +72,15 @@ public class GcsPinotFS extends BasePinotFS {
@Override
public void init(PinotConfiguration config) {
+ String version = GcsPinotFS.class.getPackage().getImplementationVersion();
+ if (version == null) {
+ version = "unknown";
+ }
+ String userAgent = "apache-pinot/:" + version + "(GPN:apache-pinot)";
Credentials credentials = null;
try {
- StorageOptions.Builder storageBuilder = StorageOptions.newBuilder();
+ StorageOptions.Builder storageBuilder = StorageOptions.newBuilder()
+ .setHeaderProvider(FixedHeaderProvider.create(ImmutableMap.of("User-agent", userAgent)));
Review Comment:
The HTTP header name 'User-agent' should be 'User-Agent' with a capital 'A'. HTTP header names are case-insensitive but the standard convention is to use proper capitalization.
```suggestion
.setHeaderProvider(FixedHeaderProvider.create(ImmutableMap.of("User-Agent", userAgent)));
```
##########
pinot-plugins/pinot-file-system/pinot-gcs/src/main/java/org/apache/pinot/plugin/filesystem/GcsPinotFS.java:
##########
@@ -70,9 +72,15 @@ public class GcsPinotFS extends BasePinotFS {
@Override
public void init(PinotConfiguration config) {
+ String version = GcsPinotFS.class.getPackage().getImplementationVersion();
+ if (version == null) {
+ version = "unknown";
+ }
+ String userAgent = "apache-pinot/:" + version + "(GPN:apache-pinot)";
Review Comment:
The User-Agent string format is incorrect. There should not be a colon after 'apache-pinot/' and before the version. The correct format should be 'apache-pinot/' + version + ' (GPN:apache-pinot)'.
```suggestion
String userAgent = "apache-pinot/" + version + " (GPN:apache-pinot)";
```
--
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] Add UserAgent in GCS requests [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #16884:
URL: https://github.com/apache/pinot/pull/16884#issuecomment-3354297052
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/16884?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 56.33%. Comparing base ([`4ba63e2`](https://app.codecov.io/gh/apache/pinot/commit/4ba63e2b8d1877cd9476ac68aebba2bcbefbbbe9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`7bd421f`](https://app.codecov.io/gh/apache/pinot/commit/7bd421f8b06feef904732913ddacc63bded94cd3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 38 commits behind head on master.
> :exclamation: There is a different number of reports uploaded between BASE (4ba63e2) and HEAD (7bd421f). Click for more details.
>
> <details><summary>HEAD has 36 uploads less than BASE</summary>
>
>| Flag | BASE (4ba63e2) | HEAD (7bd421f) |
>|------|------|------|
>|java-21|5|1|
>|unittests1|2|1|
>|unittests|4|1|
>|temurin|10|1|
>|java-11|5|0|
>|unittests2|2|0|
>|integration|6|0|
>|integration1|2|0|
>|custom-integration1|2|0|
>|integration2|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #16884 +/- ##
============================================
- Coverage 63.49% 56.33% -7.17%
+ Complexity 1410 700 -710
============================================
Files 3067 2402 -665
Lines 180097 133535 -46562
Branches 27555 21236 -6319
============================================
- Hits 114355 75221 -39134
+ Misses 56943 52038 -4905
+ Partials 8799 6276 -2523
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/16884/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/16884/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/16884/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/16884/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/16884/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/16884/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/16884/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.33% <ø> (-7.13%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/16884/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.33% <ø> (-7.17%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/16884/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.33% <ø> (-7.17%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/16884/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.33% <ø> (-0.05%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/16884/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/16884?dropdown=coverage&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).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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] Add UserAgent in GCS requests [pinot]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #16884:
URL: https://github.com/apache/pinot/pull/16884#discussion_r2393511694
##########
pinot-plugins/pinot-file-system/pinot-gcs/src/main/java/org/apache/pinot/plugin/filesystem/GcsPinotFS.java:
##########
@@ -70,9 +72,15 @@ public class GcsPinotFS extends BasePinotFS {
@Override
public void init(PinotConfiguration config) {
+ String version = GcsPinotFS.class.getPackage().getImplementationVersion();
+ if (version == null) {
+ version = "unknown";
+ }
+ String userAgent = "apache-pinot/" + version + " (GPN:apache-pinot)";
Review Comment:
[nitpick] The User-Agent construction logic should be extracted to a private method or constant to improve maintainability and testability. This also allows for easier modification of the User-Agent format in the future.
```suggestion
private static String buildUserAgent(String version) {
return "apache-pinot/" + version + " (GPN:apache-pinot)";
}
@Override
public void init(PinotConfiguration config) {
String version = GcsPinotFS.class.getPackage().getImplementationVersion();
if (version == null) {
version = "unknown";
}
String userAgent = buildUserAgent(version);
```
--
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] Add UserAgent in GCS requests [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #16884:
URL: https://github.com/apache/pinot/pull/16884#discussion_r2376012152
##########
pinot-plugins/pinot-file-system/pinot-gcs/src/main/java/org/apache/pinot/plugin/filesystem/GcsPinotFS.java:
##########
@@ -70,9 +72,15 @@ public class GcsPinotFS extends BasePinotFS {
@Override
public void init(PinotConfiguration config) {
+ String version = GcsPinotFS.class.getPackage().getImplementationVersion();
+ if (version == null) {
+ version = "unknown";
+ }
+ String userAgent = "apache-pinot/:" + version + "(GPN:apache-pinot)";
Review Comment:
@shrutisinghania can you check if this comment is true or not?
##########
pinot-plugins/pinot-file-system/pinot-gcs/src/main/java/org/apache/pinot/plugin/filesystem/GcsPinotFS.java:
##########
@@ -70,9 +72,15 @@ public class GcsPinotFS extends BasePinotFS {
@Override
public void init(PinotConfiguration config) {
+ String version = GcsPinotFS.class.getPackage().getImplementationVersion();
+ if (version == null) {
+ version = "unknown";
+ }
+ String userAgent = "apache-pinot/:" + version + "(GPN:apache-pinot)";
Review Comment:
@shrutisinghania can you check if this comment is valid or not?
--
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] Add UserAgent in GCS requests [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #16884:
URL: https://github.com/apache/pinot/pull/16884#discussion_r2393113493
##########
pinot-plugins/pinot-file-system/pinot-gcs/src/main/java/org/apache/pinot/plugin/filesystem/GcsPinotFS.java:
##########
@@ -70,9 +72,15 @@ public class GcsPinotFS extends BasePinotFS {
@Override
public void init(PinotConfiguration config) {
+ String version = GcsPinotFS.class.getPackage().getImplementationVersion();
+ if (version == null) {
+ version = "unknown";
+ }
+ String userAgent = "apache-pinot/" + version + " (GPN:apache-pinot)";
Credentials credentials = null;
try {
- StorageOptions.Builder storageBuilder = StorageOptions.newBuilder();
+ StorageOptions.Builder storageBuilder = StorageOptions.newBuilder()
+ .setHeaderProvider(FixedHeaderProvider.create(ImmutableMap.of("User-agent", userAgent)));
Review Comment:
(minor) Suggest using `Map.of()`
```suggestion
.setHeaderProvider(FixedHeaderProvider.create(Map.of("User-agent", userAgent)));
```
--
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