Skip to content

[improve][doc] Add info for health check CMD and fix incorrect REST API links - #16267

Merged
Anonymitaet merged 6 commits into
apache:masterfrom
momo-jun:quick-fix-0627
Jul 7, 2022
Merged

[improve][doc] Add info for health check CMD and fix incorrect REST API links#16267
Anonymitaet merged 6 commits into
apache:masterfrom
momo-jun:quick-fix-0627

Conversation

@momo-jun

@momo-jun momo-jun commented Jun 28, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Modifications

  1. Fix Need an api interface for the process to start working normally for rolling restart of pulsar #14767 - Add a tip to the Upgrade doc page for how to check the health of a broker via Pulsar-admin and REST API.
  2. Minor fixes on the Upgrade doc page.
  3. Fix incorrect REST API links on more doc pages.

The preview looks good.
image

Documentation

  • doc

@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 Jun 28, 2022
@momo-jun momo-jun changed the title [improve][doc] add info for health check [improve][doc] Add info for health check CMD and fix incorrect REST API links Jun 29, 2022
@momo-jun
momo-jun marked this pull request as ready for review June 29, 2022 07:42
@momo-jun

momo-jun commented Jun 29, 2022

Copy link
Copy Markdown
Contributor Author

Hi @horizonzy, thanks for your support and effort in addressing the REST API issues. Can you pls review this PR?

horizonzy
horizonzy approved these changes Jun 29, 2022
View reviewed changes

@horizonzy horizonzy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

@momo-jun

momo-jun commented Jun 29, 2022

Copy link
Copy Markdown
Contributor Author

Ping @Anonymitaet for review.

Anonymitaet
Anonymitaet approved these changes Jun 30, 2022
View reviewed changes
Comment thread site2/docs/admin-api-brokers.md
<TabItem value="REST API">

{@inject: endpoint|GET|/admin/v2/brokers/leaderBroker?version=@pulsar:version_number@}
{@inject: endpoint|GET|/admin/v2/brokers/leaderBroker|operation/getLeaderBroker?version=@pulsar:version_number@}

@Anonymitaet Anonymitaet Jun 30, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. Do these links go to the specific REST API doc rather than the home page?
  2. Will you fix all the occurrences in master?

@momo-jun momo-jun Jun 30, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  1. Yes. The logic has been verified. Theoretically, they can go to the correct place but this cannot be directly verified in the local build because what we use for version numbers is a variable.
  2. I've corrected all the occurrences that I've searched out in the master branch.

@Anonymitaet Anonymitaet Jun 30, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

for issue 2, I mean we have some incorrect REST API links in the whole Pulsar doc, will you fix them all (on master)?

Example:
image

@momo-jun momo-jun Jun 30, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh, I see your point. I didn't expect we have such cases. The search term I use is "@Pulsar:version_number@", to make sure it is injected in the correct place. So these cases are not included. I can resolve them in another PR after verifying the results of this PR on the Pulsar website.

@momo-jun momo-jun Jun 30, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

After looking into the MD file, I found the link itself is correct, which contains "@Pulsar:version_number@" and the variable is replaced with the version number of docs as we can see on the website. The only issue is that it is written in a command style, rather than a link. I will commit the updates today.

@Anonymitaet Anonymitaet Jun 30, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK, thanks!

@momo-jun momo-jun Jun 30, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@Anonymitaet the updates for the cmd style have been committed.

Comment thread site2/website/versioned_sidebars/version-2.8.3-sidebars.json
{
"type": "doc",
"id": "version-2.8.2/security-basic-auth"
"id": "version-2.8.3/security-basic-auth"

@Anonymitaet Anonymitaet Jun 30, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

reminder: any other doc versions need this chapter?

@momo-jun momo-jun Jun 30, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a fix. Other doc versions require this chapter have been implemented in #15734.

@Anonymitaet Anonymitaet added this to the 2.11.0 milestone Jun 30, 2022
@Anonymitaet Anonymitaet added doc-complete Your PR changes impact docs and the related docs have been already added. doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. and removed doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. doc-complete Your PR changes impact docs and the related docs have been already added. labels Jun 30, 2022
@Anonymitaet
Anonymitaet merged commit cd9eaca into apache:master Jul 7, 2022
@momo-jun momo-jun mentioned this pull request Jul 8, 2022
Merged
1 task
zymap pushed a commit to zymap/pulsar that referenced this pull request Jul 11, 2022
@momo-jun @zymap
[improve][doc] Add info for health check CMD and fix incorrect REST A…
76baeee 
…PI links (apache#16267)
wuxuanqicn pushed a commit to wuxuanqicn/pulsar that referenced this pull request Jul 14, 2022
@momo-jun @weimob-wuxuanqi
[improve][doc] Add info for health check CMD and fix incorrect REST A…
0a38d2c 
…PI links (apache#16267)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@horizonzy horizonzy horizonzy approved these changes
@Anonymitaet Anonymitaet Anonymitaet approved these changes

Assignees

No one assigned

Labels

doc Your PR contains doc changes, no matter whether the changes are in markdown or code files.

Projects

None yet

Milestone

2.11.0

Development

Successfully merging this pull request may close these issues.

Need an api interface for the process to start working normally for rolling restart of pulsar

3 participants

@momo-jun @horizonzy @Anonymitaet