-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Add readiness api for the worker leader #7601
Conversation
@ApiResponse(code = 401, message = "The requester is not authenticated"), | ||
@ApiResponse(code = 503, message = "Worker service is not running") | ||
}) | ||
@Path("/cluster/leaderready") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the url path would be more clean if it is /cluster/leader/ready. We can add more API under "/leader" in future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beside we already have have the url path "/cluster/leader"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed
@srkukarni what is the purpose of adding this new REST endpoint? |
@jerrypeng the current usecase is for setting up testing pipelines. |
}) | ||
@Path("/cluster/leader/ready") | ||
@Produces(MediaType.APPLICATION_JSON) | ||
public String isLeaderReady() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is more appropriate to just return 200 for OK or 503 (service not available). That is more idiomatic for HTTP REST calls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed
So this endpoint is authentication protected, is that what we want? |
/pulsarbot run-failure-checks |
/pulsarbot run-failure-checks |
1 similar comment
/pulsarbot run-failure-checks |
* Added upgrade notes * Add new end point to verify leader readiness * Address feedback * Address feedback * Add new end point to verify leader readiness * Address feedback * Address feedback Co-authored-by: Sanjeev Kulkarni <sanjeevk@splunk.com> (cherry picked from commit b5aa109)
* Added upgrade notes * Add new end point to verify leader readiness * Address feedback * Address feedback * Add new end point to verify leader readiness * Address feedback * Address feedback Co-authored-by: Sanjeev Kulkarni <sanjeevk@splunk.com>
* Added upgrade notes * Add new end point to verify leader readiness * Address feedback * Address feedback * Add new end point to verify leader readiness * Address feedback * Address feedback Co-authored-by: Sanjeev Kulkarni <sanjeevk@splunk.com>
(If this PR fixes a github issue, please add
Fixes #<xyz>
.)Fixes #
(or if this PR is one task of a github issue, please add
Master Issue: #<xyz>
to link to the master issue.)Master Issue: #
Motivation
This mr adds an api to check if the worker is ready to serve requests.
Modifications
Describe the modifications you've done.
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If
yes
was chosen, please highlight the changesDocumentation