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

Update doc of listen-client-urls and listen-client-http-urls #866

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

Sadamingh
Copy link
Contributor

PR for issue: #777 to - doc on listen-client-http-urls for gRPC and HTTP seperation.
Original discussions: : etcd-io/etcd#15446

@Sadamingh
Copy link
Contributor Author

Sadamingh commented Jun 24, 2024

/cc @jmhbnz
Thanks

@k8s-ci-robot k8s-ci-robot requested a review from jmhbnz June 24, 2024 07:34
@k8s-ci-robot
Copy link

@Sadamingh: GitHub didn't allow me to request PR reviews from the following users: Thanks.

Note that only etcd-io members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @jmhbnz Thanks

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

Thanks for raising this @Sadamingh, looks good, just need to update the other docs versions mentioned below as well.

@Sadamingh
Copy link
Contributor Author

Update: added --listen-client-http-urls flag directions to doc v3.4 and v3.6
Modify: Changed default port of http urls to 2383 according to etcd-io/etcd/tests/framework/e2e/cluster.go#L510
/cc @jmhbnz

@k8s-ci-robot k8s-ci-robot requested a review from jmhbnz June 24, 2024 17:56
Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

Thanks @Sadamingh - One last issue to fix for default value then this will be good to merge.

content/en/docs/v3.4/op-guide/configuration.md Outdated Show resolved Hide resolved
content/en/docs/v3.5/op-guide/configuration.md Outdated Show resolved Hide resolved
content/en/docs/v3.6/op-guide/configuration.md Outdated Show resolved Hide resolved
@Sadamingh Sadamingh force-pushed the listenclienthttpurls-flag-doc branch from 427b1fd to 2911f66 Compare June 30, 2024 17:46
@Sadamingh
Copy link
Contributor Author

Sadamingh commented Jun 30, 2024

Thanks @jmhbnz , cleaned up default port for this flag.

@Sadamingh Sadamingh requested a review from jmhbnz June 30, 2024 17:49
Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks @Sadamingh

@jmhbnz jmhbnz requested a review from spzala June 30, 2024 20:33
@jmhbnz jmhbnz requested a review from ivanvc July 8, 2024 08:13
Copy link
Member

@ivanvc ivanvc left a comment

Choose a reason for hiding this comment

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

Thanks, @Sadamingh. This looks great; I left two minor possible improvements.

content/en/docs/v3.4/op-guide/configuration.md Outdated Show resolved Hide resolved
content/en/docs/v3.4/op-guide/configuration.md Outdated Show resolved Hide resolved
Signed-off-by: Sadamingh <yxing11@usfca.edu>
@Sadamingh
Copy link
Contributor Author

Thanks @ivanvc, these code formattings are added.
/cc @spzala

Copy link
Member

@spzala spzala left a comment

Choose a reason for hiding this comment

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

Thanks @Sadamingh lgtm!

Copy link
Member

@ivanvc ivanvc left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks, @Sadamingh

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ivanvc, jmhbnz, Sadamingh, spzala

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@spzala spzala merged commit 65d06b4 into etcd-io:main Jul 10, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants