You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/06/28 10:44:04 UTC
[GitHub] [pulsar] momo-jun opened a new pull request, #16267: [improve][doc] add info for health check
momo-jun opened a new pull request, #16267:
URL: https://github.com/apache/pulsar/pull/16267
### Modifications
1. Fix #14767 - Add a tip to the Upgrade page for broker health check.
<img width="1269" alt="image" src="https://user-images.githubusercontent.com/60642177/176159224-0be6f4de-dec4-47f6-b3df-d08b902d8668.png">
2. Minor fixes.
### Documentation
- [ ] `doc`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on a diff in pull request #16267: [improve][doc] Add info for health check CMD and fix incorrect REST API links
Posted by GitBox <gi...@apache.org>.
momo-jun commented on code in PR #16267:
URL: https://github.com/apache/pulsar/pull/16267#discussion_r910530659
##########
site2/docs/admin-api-brokers.md:
##########
@@ -128,7 +128,7 @@ BrokerInfo(serviceUrl=broker1.use.org.com:8080)
</TabItem>
<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@}
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on pull request #16267: [improve][doc] Add info for health check CMD and fix incorrect REST API links
Posted by GitBox <gi...@apache.org>.
momo-jun commented on PR #16267:
URL: https://github.com/apache/pulsar/pull/16267#issuecomment-1169645129
Hi @horizonzy, thanks for your support and effort in addressing the REST API issues. Can you pls review this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on a diff in pull request #16267: [improve][doc] Add info for health check CMD and fix incorrect REST API links
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on code in PR #16267:
URL: https://github.com/apache/pulsar/pull/16267#discussion_r910569203
##########
site2/docs/admin-api-brokers.md:
##########
@@ -128,7 +128,7 @@ BrokerInfo(serviceUrl=broker1.use.org.com:8080)
</TabItem>
<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@}
Review Comment:
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:
<img width="1477" alt="image" src="https://user-images.githubusercontent.com/50226895/176585398-c5236146-ec20-430f-b03e-9e365cb32ec9.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on a diff in pull request #16267: [improve][doc] Add info for health check CMD and fix incorrect REST API links
Posted by GitBox <gi...@apache.org>.
momo-jun commented on code in PR #16267:
URL: https://github.com/apache/pulsar/pull/16267#discussion_r910530256
##########
site2/website/versioned_sidebars/version-2.8.3-sidebars.json:
##########
@@ -384,7 +384,7 @@
},
{
"type": "doc",
- "id": "version-2.8.2/security-basic-auth"
+ "id": "version-2.8.3/security-basic-auth"
Review Comment:
This is a fix. Other doc versions require this chapter have been implemented in #15734.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on a diff in pull request #16267: [improve][doc] Add info for health check CMD and fix incorrect REST API links
Posted by GitBox <gi...@apache.org>.
momo-jun commented on code in PR #16267:
URL: https://github.com/apache/pulsar/pull/16267#discussion_r910626427
##########
site2/docs/admin-api-brokers.md:
##########
@@ -128,7 +128,7 @@ BrokerInfo(serviceUrl=broker1.use.org.com:8080)
</TabItem>
<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@}
Review Comment:
After looking to 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 in the website. The only issue is: it is written in a command style, rather than a link.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on a diff in pull request #16267: [improve][doc] Add info for health check CMD and fix incorrect REST API links
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on code in PR #16267:
URL: https://github.com/apache/pulsar/pull/16267#discussion_r910569203
##########
site2/docs/admin-api-brokers.md:
##########
@@ -128,7 +128,7 @@ BrokerInfo(serviceUrl=broker1.use.org.com:8080)
</TabItem>
<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@}
Review Comment:
for issue 2, I mean we have many REST API links in the whole Pulsar doc, will you fix them all (on `master`)?
Example:
<img width="1377" alt="image" src="https://user-images.githubusercontent.com/50226895/176585165-600f2435-d473-4eb8-9c0d-1e7f71bdef34.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on a diff in pull request #16267: [improve][doc] Add info for health check CMD and fix incorrect REST API links
Posted by GitBox <gi...@apache.org>.
momo-jun commented on code in PR #16267:
URL: https://github.com/apache/pulsar/pull/16267#discussion_r910575361
##########
site2/docs/admin-api-brokers.md:
##########
@@ -128,7 +128,7 @@ BrokerInfo(serviceUrl=broker1.use.org.com:8080)
</TabItem>
<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@}
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on a diff in pull request #16267: [improve][doc] Add info for health check CMD and fix incorrect REST API links
Posted by GitBox <gi...@apache.org>.
momo-jun commented on code in PR #16267:
URL: https://github.com/apache/pulsar/pull/16267#discussion_r910626427
##########
site2/docs/admin-api-brokers.md:
##########
@@ -128,7 +128,7 @@ BrokerInfo(serviceUrl=broker1.use.org.com:8080)
</TabItem>
<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@}
Review Comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on a diff in pull request #16267: [improve][doc] Add info for health check CMD and fix incorrect REST API links
Posted by GitBox <gi...@apache.org>.
momo-jun commented on code in PR #16267:
URL: https://github.com/apache/pulsar/pull/16267#discussion_r910739574
##########
site2/docs/admin-api-brokers.md:
##########
@@ -128,7 +128,7 @@ BrokerInfo(serviceUrl=broker1.use.org.com:8080)
</TabItem>
<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@}
Review Comment:
@Anonymitaet the updates for the cmd style have been committed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on pull request #16267: [improve][doc] Add info for health check CMD and fix incorrect REST API links
Posted by GitBox <gi...@apache.org>.
momo-jun commented on PR #16267:
URL: https://github.com/apache/pulsar/pull/16267#issuecomment-1169928180
Ping @Anonymitaet for review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on a diff in pull request #16267: [improve][doc] Add info for health check CMD and fix incorrect REST API links
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on code in PR #16267:
URL: https://github.com/apache/pulsar/pull/16267#discussion_r910514295
##########
site2/docs/admin-api-brokers.md:
##########
@@ -128,7 +128,7 @@ BrokerInfo(serviceUrl=broker1.use.org.com:8080)
</TabItem>
<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@}
Review Comment:
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`?
##########
site2/website/versioned_sidebars/version-2.8.3-sidebars.json:
##########
@@ -384,7 +384,7 @@
},
{
"type": "doc",
- "id": "version-2.8.2/security-basic-auth"
+ "id": "version-2.8.3/security-basic-auth"
Review Comment:
reminder: any other doc versions need this chapter?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet merged pull request #16267: [improve][doc] Add info for health check CMD and fix incorrect REST API links
Posted by GitBox <gi...@apache.org>.
Anonymitaet merged PR #16267:
URL: https://github.com/apache/pulsar/pull/16267
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org