Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix][fn] Make /version return correct version #20047

Merged

Conversation

michaeljmarshall
Copy link
Member

Motivation

The /version endpoint for functions currently returns {"version":"version.number"} instead of an actual version number. This PR fixes that.

One observation is that the broker's /version endpoint returns a plain string:

@GET
@Path("/version")
@ApiOperation(value = "Get version of current broker")
@ApiResponses(value = {
@ApiResponse(code = 200, message = "Everything is OK"),
@ApiResponse(code = 500, message = "Internal server error")})
public String version() throws Exception {
return PulsarVersion.getVersion();
}

Modifications

  • Return the actual version.
  • Use a static field to decrease the cost of this endpoint.

Verifying this change

This is a trivial change.

Documentation

  • doc-not-needed

This fixes an endpoint, no docs are needed.

Matching PR in forked repository

PR in forked repository: Skipping PR since this is so trivial

@michaeljmarshall michaeljmarshall added type/bug The PR fixed a bug or issue reported a bug area/function doc-not-needed Your PR changes do not impact docs ready-to-test labels Apr 8, 2023
@michaeljmarshall michaeljmarshall added this to the 3.0.0 milestone Apr 8, 2023
@michaeljmarshall michaeljmarshall self-assigned this Apr 8, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #20047 (b28f95c) into master (3b118b6) will increase coverage by 0.05%.
The diff coverage is 44.39%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #20047      +/-   ##
============================================
+ Coverage     72.86%   72.91%   +0.05%     
- Complexity    31628    31649      +21     
============================================
  Files          1861     1858       -3     
  Lines        137500   137420      -80     
  Branches      15141    15107      -34     
============================================
+ Hits         100187   100203      +16     
+ Misses        29351    29236     -115     
- Partials       7962     7981      +19     
Flag Coverage Δ
inttests 24.42% <0.95%> (+0.02%) ⬆️
systests 25.01% <20.04%> (-0.01%) ⬇️
unittests 72.18% <35.32%> (+0.05%) ⬆️

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

Impacted Files Coverage Δ
...a/org/apache/pulsar/broker/admin/v2/Functions.java 0.00% <0.00%> (ø)
...java/org/apache/pulsar/broker/admin/v2/Worker.java 11.76% <0.00%> (ø)
...org/apache/pulsar/broker/admin/v2/WorkerStats.java 40.00% <0.00%> (ø)
...r/functions/worker/rest/ConfigurationResource.java 0.00% <0.00%> (ø)
...ons/worker/rest/api/v2/FunctionsApiV2Resource.java 0.00% <0.00%> (ø)
...s/worker/rest/api/v2/WorkerStatsApiV2Resource.java 0.00% <0.00%> (ø)
...ulsar/functions/worker/rest/api/FunctionsImpl.java 56.55% <16.66%> (-0.74%) ⬇️
...org/apache/pulsar/broker/admin/impl/SinksBase.java 35.29% <35.71%> (ø)
...g/apache/pulsar/broker/admin/impl/SourcesBase.java 35.29% <35.71%> (-1.85%) ⬇️
...ons/worker/rest/api/v3/FunctionsApiV3Resource.java 43.39% <37.50%> (ø)
... and 16 more

... and 63 files with indirect coverage changes

@michaeljmarshall michaeljmarshall merged commit 90b9dd4 into apache:master Apr 8, 2023
@michaeljmarshall michaeljmarshall deleted the fix-function-worker-version branch April 8, 2023 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/function doc-not-needed Your PR changes do not impact docs ready-to-test type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants