Skip to content

[fix][broker]fix missing return when internalGetReplicatedSubscriptionStatus#19054

Merged
congbobo184 merged 1 commit into
apache:masterfrom
HQebupt:missReturn
Dec 27, 2022
Merged

[fix][broker]fix missing return when internalGetReplicatedSubscriptionStatus#19054
congbobo184 merged 1 commit into
apache:masterfrom
HQebupt:missReturn

Conversation

@HQebupt

@HQebupt HQebupt commented Dec 25, 2022

Copy link
Copy Markdown
Contributor

Motivation

Fix missing return null if throw any exception when internalGetReplicatedSubscriptionStatus

Modifications

Add return null when handing exception in internalGetReplicatedSubscriptionStatus.

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

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

Matching PR in forked repository

PR in forked repository: HQebupt#7

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Dec 25, 2022
@HQebupt

HQebupt commented Dec 26, 2022

Copy link
Copy Markdown
Contributor Author

/pulsarbot rerun-failure-checks

@codecov-commenter

codecov-commenter commented Dec 26, 2022

Copy link
Copy Markdown

Codecov Report

Merging #19054 (facfb84) into master (d8569cd) will decrease coverage by 0.21%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #19054      +/-   ##
============================================
- Coverage     47.20%   46.98%   -0.22%     
+ Complexity    10645    10594      -51     
============================================
  Files           709      709              
  Lines         69421    69422       +1     
  Branches       7449     7449              
============================================
- Hits          32769    32620     -149     
- Misses        32984    33120     +136     
- Partials       3668     3682      +14     
Flag Coverage Δ
unittests 46.98% <100.00%> (-0.22%) ⬇️

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

Impacted Files Coverage Δ
...pulsar/broker/admin/impl/PersistentTopicsBase.java 59.41% <100.00%> (-0.12%) ⬇️
...java/org/apache/pulsar/proxy/stats/TopicStats.java 58.82% <0.00%> (-41.18%) ⬇️
...rg/apache/pulsar/broker/lookup/v1/TopicLookup.java 60.00% <0.00%> (-13.34%) ⬇️
...e/pulsar/broker/service/EntryBatchIndexesAcks.java 82.14% <0.00%> (-10.72%) ⬇️
...ava/org/apache/pulsar/broker/service/Consumer.java 66.54% <0.00%> (-5.34%) ⬇️
...g/apache/pulsar/client/impl/ConnectionHandler.java 50.00% <0.00%> (-5.32%) ⬇️
...he/pulsar/client/impl/PartitionedProducerImpl.java 30.34% <0.00%> (-5.13%) ⬇️
...oker/service/schema/SchemaRegistryServiceImpl.java 61.84% <0.00%> (-4.92%) ⬇️
...sar/broker/service/schema/SchemaRegistryStats.java 75.00% <0.00%> (-2.50%) ⬇️
...rg/apache/pulsar/broker/web/PulsarWebResource.java 56.31% <0.00%> (-2.03%) ⬇️
... and 25 more

@liangyepianzhou liangyepianzhou left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested changes:

 if (exception != null) {
 ...
} else {
...
}
return null;

@gaoran10 gaoran10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice catch!

@congbobo184 congbobo184 merged commit 3df9ecf into apache:master Dec 27, 2022
tisonkun pushed a commit to tisonkun/pulsar that referenced this pull request Dec 27, 2022
…nStatus (apache#19054)

### Motivation

Fix missing `return null` if throw any exception when `internalGetReplicatedSubscriptionStatus`

### Modifications

Add `return null` when handing exception in `internalGetReplicatedSubscriptionStatus`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/broker doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants