-
Notifications
You must be signed in to change notification settings - Fork 9k
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
YARN-11794. hadoop-yarn-csi does not build on included Centos 7 docker image #7499
base: branch-3.4
Are you sure you want to change the base?
Conversation
…r image also fix Centos 7 docker image build
💔 -1 overall
This message was automatically generated. |
@@ -27,6 +27,8 @@ | |||
|
|||
<properties> | |||
<grpc.version>1.69.0</grpc.version> | |||
<!-- Last version that works with Centos 7 --> | |||
<grpc.plugin.version>1.65.0</grpc.plugin.version> |
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 looks risky to use a different gRPC version for codegen and runtime, an alternative I can imagine -track the generated source code in Git, and only regenerate the code on each gRPC upgrading
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.
There are no code changes in the compiler between those versions , @pan3793
Only the build OS is changed, event the netty version change is reverted:
https://github.com/grpc/grpc-java/commits/master/compiler
The incompatibility is caused by
grpc/grpc-java@71eb5fb
committing generated code to git is troublesome, I would very much like to avoid that.
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.
There are no code changes in the compiler between those versions
it's fine for these specific version pair, but may not be good for future upgrading
committing generated code to git is troublesome, I would very much like to avoid that.
I tend to agree with you in most cases, but I think for this case, it's a most safe and relatively cheap way.
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's fine for these specific version pair, but may not be good for future upgrading
We can cross that rover when we get there.
If you REALLY don't want to use different versions, then I'd preder recompiling the plugin from source on Centos 7, though I'd hate to put in that much effort to support EOL distros.
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.
Another way used by Spark, allow user to configure pre-installed protoc
/grpc
commands to generate the code, then we can compile and install those commands from the source code on the CentOS 7 build image (I'm not familiar with the native world, so have no idea how much maintenance effort will be paid for this way)
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.
Yes, that's what I meant by recompiling the grpc compiler from source.
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.
BTW, Hadoop actually has committed a few generated code into Git repo
https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/arm-java
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.
Thanks to both of you for the discussion! In the current situation, I lean towards supporting Story's approach, as long as it doesn't block the progress of Hadoop 3.4.2. In fact, we don't actually need this CentOS7 script during the release process. If a similar issue arises in a future version of Ubuntu, I believe we can follow @pan3793 method to fix it.
also fix Centos 7 docker image build
Description of PR
Revert io.grpc:protoc-gen-grpc-java to 1.65.0 to enable building on Centos 7.
Also backported Centos 7 image build fix from trunk.
How was this patch tested?
Started Centos7 container.
ran 'mvn clean package -DskipTests'
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?