Skip to content

Conversation

@BewareMyPower
Copy link
Contributor

Motivation

#19705

Modifications

  • Add the ClientBuilderImpl#description method to add the description to the original client version string that is set in CommandConnect and CommandAuthResponse.
  • Add testClientVersion to cover these two cases.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: BewareMyPower#21

@BewareMyPower BewareMyPower added the type/feature The PR added a new feature or issue requested a new feature label Apr 4, 2023
@BewareMyPower BewareMyPower added this to the 3.0.0 milestone Apr 4, 2023
@BewareMyPower BewareMyPower self-assigned this Apr 4, 2023
@github-actions github-actions bot added the doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. label Apr 4, 2023
### Motivation

apache#19705

### Modifications

- Add the `ClientBuilderImpl#description` method to add the description
  to the original client version string that is set in `CommandConnect`
  and `CommandAuthResponse`.
- Add `testClientVersion` to cover these two cases.
@BewareMyPower BewareMyPower force-pushed the bewaremypower/pip-254-client-version branch from 2114f3a to 9ea0d23 Compare April 6, 2023 15:07
Copy link
Member

@RobertIndie RobertIndie left a comment

Choose a reason for hiding this comment

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

+1 for the new change.

@codecov-commenter
Copy link

Codecov Report

Merging #20009 (927064b) into master (dd05408) will increase coverage by 38.22%.
The diff coverage is 80.89%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #20009       +/-   ##
=============================================
+ Coverage     34.65%   72.88%   +38.22%     
- Complexity    12429    31673    +19244     
=============================================
  Files          1606     1858      +252     
  Lines        125026   137532    +12506     
  Branches      13667    15129     +1462     
=============================================
+ Hits          43332   100240    +56908     
+ Misses        76081    29319    -46762     
- Partials       5613     7973     +2360     
Flag Coverage Δ
inttests 24.24% <2.24%> (-0.14%) ⬇️
systests 25.00% <2.24%> (?)
unittests 72.18% <80.89%> (+39.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...tent/NonPersistentDispatcherMultipleConsumers.java 71.60% <ø> (+32.09%) ⬆️
...g/apache/pulsar/client/impl/ClientBuilderImpl.java 84.76% <50.00%> (+35.78%) ⬆️
...uth/KubernetesServiceAccountTokenAuthProvider.java 77.27% <77.27%> (ø)
...ervice/AbstractDispatcherSingleActiveConsumer.java 92.37% <100.00%> (+13.74%) ⬆️
...ersistentStickyKeyDispatcherMultipleConsumers.java 76.71% <100.00%> (+19.35%) ⬆️
...ersistentStickyKeyDispatcherMultipleConsumers.java 85.09% <100.00%> (+3.32%) ⬆️
.../java/org/apache/pulsar/client/impl/ClientCnx.java 71.48% <100.00%> (+22.49%) ⬆️
...lsar/client/impl/conf/ClientConfigurationData.java 96.63% <100.00%> (+8.50%) ⬆️
...functions/auth/KubernetesFunctionAuthProvider.java 80.00% <100.00%> (+80.00%) ⬆️
...s/runtime/kubernetes/KubernetesRuntimeFactory.java 80.42% <100.00%> (+80.42%) ⬆️
... and 1 more

... and 1495 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. ready-to-test type/feature The PR added a new feature or issue requested a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants