[improve][client] Allow adding custom description to User-Agent header #24729
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR extends the User-Agent header customization feature (originally implemented for pulsar-client in PIP-254) to the pulsar-admin component, allowing users to add custom descriptions to identify their applications.
- Updates User-Agent construction in both pulsar-client and pulsar-admin to append client description when provided
- Modifies the User-Agent format from "Pulsar-Java-v{version}" to "Pulsar-Java-v{version}-{description}" when description is set
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pulsar-client/src/main/java/org/apache/pulsar/client/impl/HttpClient.java | Updates User-Agent construction to include optional client description |
| pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/http/AsyncHttpConnector.java | Updates User-Agent construction to include optional client description |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
b6781b9 to
144a822
Compare
144a822 to
f20704b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #24729 +/- ##
============================================
- Coverage 74.27% 74.16% -0.11%
+ Complexity 33569 33503 -66
============================================
Files 1896 1896
Lines 148111 148117 +6
Branches 17164 17166 +2
============================================
- Hits 110009 109856 -153
- Misses 29370 29460 +90
- Partials 8732 8801 +69
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| public PulsarAdminBuilder description(String description) { | ||
| this.conf.setDescription(description); | ||
| return this; | ||
| } |
There was a problem hiding this comment.
Where is the check for the max 64 character limit? Please add a test that it's not allowed.
There was a problem hiding this comment.
I will make a PR to add this test.
There was a problem hiding this comment.
@nodece It seems that you merged without checking this comment
I didn't see your comment when I replied
There was a problem hiding this comment.
@BewareMyPower Good catch. I should add the character limit in the builder.
There was a problem hiding this comment.
@lhotari @BewareMyPower I submitted #24734 to fix this.
apache#24729) (cherry picked from commit e19856a) Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Motivation
PIP-254 only improved the pulsar-client. The pulsar-admin was not covered. This PR extends that work to also support the pulsar-admin. While not explicitly included in PIP-254, this can be considered a natural part of it.
Modifications
description(String)to thePulsarAdminBuilderandPulsarAdminBuilderImplDocumentation
docdoc-requireddoc-not-neededdoc-complete