Skip to content

Conversation

gnossen
Copy link
Contributor

@gnossen gnossen commented May 4, 2021

@sanjaypujare Does this need to be backported to the v1.37.x branch as well?

@gnossen gnossen added area/documentation lang/Python release notes: no Indicates if PR should not be in release notes labels May 4, 2021
@gnossen gnossen requested review from yashykt and sanjaypujare May 4, 2021 21:51
@sanjaypujare
Copy link
Contributor

@sanjaypujare Does this need to be backported to the v1.37.x branch as well?

Yes. Ideally we will mention a release tag to fetch the example and that will be a 1.37.n (it will have to be 1.37.2 when it does come out)

@yashykt
Copy link
Member

yashykt commented May 4, 2021

I don't think backporting it is necessary. Why can't we just have it work from head?

@gnossen
Copy link
Contributor Author

gnossen commented May 4, 2021

I don't think backporting it is necessary. Why can't we just have it work from head?

One issue is that master is not a permalink. The examples directory may move in the future.

Yes. Ideally we will mention a release tag to fetch the example and that will be a 1.37.n (it will have to be 1.37.2 when it does come out)

To the best of my knowledge, this example is the only thing that would require a patch release, which involves a full release of binary artifacts. Would it not be sufficient to just link to the v1.37.x branch? Alternatively, we could link to a specific commit.

@sanjaypujare
Copy link
Contributor

I don't think backporting it is necessary. Why can't we just have it work from head?

We are going to document this in the UG and if the head changes (for whatever reason) we still want the documented examples links in the UG to work. The probability is very very small that head won't work can't be guaranteed.

@sanjaypujare
Copy link
Contributor

To the best of my knowledge, this example is the only thing that would require a patch release, which involves a full release of binary artifacts. Would it not be sufficient to just link to the v1.37.x branch?

There are no release artifacts related to the v1.37.x branch but there are for an actual release like v1.37.1 and so on. With the artifacts you just download the zip file and be done with it. With a branch you will do a git clone?

@gnossen
Copy link
Contributor Author

gnossen commented May 4, 2021

With a branch you will do a git clone?

Branches, commits, and tags can all be downloaded from GitHub as zip/tarball over HTTP. For example:

https://github.com/grpc/grpc/archive/refs/heads/master.tar.gz
https://github.com/grpc/grpc/archive/9b87fb33bf61847ca05deb788532c2a182e93b26.tar.gz
https://github.com/grpc/grpc/archive/refs/tags/v1.37.1.tar.gz

My question was really about whether we really need to go through the whole patch release process for this?

@sanjaypujare
Copy link
Contributor

My question was really about whether we really need to go through the whole patch release process for this?

No.

Copy link
Member

@yashykt yashykt left a comment

Choose a reason for hiding this comment

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

The only issue I see is that we don't have a maintenance port for health checking. What we want to have is the server listening on two ports always. for example, 50051 and 50052. 50051 (secure/insecure) would be the port on which the server actually serves greeter requests while 50052 (always insecure) would be just for health checking.

@gnossen
Copy link
Contributor Author

gnossen commented May 4, 2021

@yashykt Nice catch! Should be fixed now.

Copy link
Contributor

@sanjaypujare sanjaypujare left a comment

Choose a reason for hiding this comment

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

LG

@gnossen
Copy link
Contributor Author

gnossen commented May 5, 2021

@gnossen gnossen merged commit 7125bbe into grpc:master May 5, 2021
gnossen added a commit to gnossen/grpc that referenced this pull request May 6, 2021
* Add Python PSM security example

* Fix lint

* Add in maintenance port configuration

* Align CLI flags with Java

* Pylint
paulosjca pushed a commit to paulosjca/grpc that referenced this pull request May 7, 2021
* Add Python PSM security example

* Fix lint

* Add in maintenance port configuration

* Align CLI flags with Java

* Pylint
gnossen added a commit that referenced this pull request May 7, 2021
* Add Python PSM security example

* Fix lint

* Add in maintenance port configuration

* Align CLI flags with Java

* Pylint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation lang/Python release notes: no Indicates if PR should not be in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants