-
Notifications
You must be signed in to change notification settings - Fork 3.9k
xds: add support for setting bootstrap file with java system property #7620
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
Conversation
|
|
Blocked waiting for Corporate CLA approval. |
|
@ejona86 @sanjaypujare FYI the link here https://github.com/grpc/grpc-java/blob/master/CONTRIBUTING.md#legal-requirements (https://identity.linuxfoundation.org/projects/cncf) goes to the old and now outdated system. |
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.
It seems like maybe we don't need to mention the system property in the example, but we don't actually have another place to put that documentation so it seems best for now.
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.
Added 2 comments but looks good.
While most languages support setting environment variables during runtime, Java does not. In Java, the preferred approach is to use Java System Properties in order so specify configuration options. By checking for the existence of the io.grpc.xds.bootstrap property if GRPC_XDS_BOOTSTRAP is not found, it is possible to either supply the bootstrap location during runtime or as a Java argument. The environment variable still takes precedence in order to not break any existing documentation.
f5065b5
to
6ff16a0
Compare
@erikjoh it is better if you don't squash commits and "force-push" them - that way it is easier to see the changes made in the second commit. While merging the commits can be squashed. |
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.
LG
Ah sorry, I don't use the squash merge feature a lot! Noted for the next time 🙂 |
@sanjaypujare That build failure seems unrelated to this since the class was introduced recently and does not exist on my branch even. Doesn't seem like I have any option to rerun the build.
|
Thank you! |
While most languages support setting environment variables during runtime,
Java does not. In Java, the preferred approach is to use Java System Properties
in order so specify configuration options. By checking for the existence
of the io.grpc.xds.bootstrap property if GRPC_XDS_BOOTSTRAP is not found,
it is possible to either supply the bootstrap location during runtime or as
a Java argument. The environment variable still takes precedence in order
to not break any existing documentation.
For discussion see #7605 .