fix: S2A - skip S2A on platforms where netty-tcnative is not available#3802
fix: S2A - skip S2A on platforms where netty-tcnative is not available#3802rmehta19 wants to merge 3 commits intogoogleapis:mainfrom
Conversation
|
@lqiu96 , would you be able to review this when you get a chance? Thank you! |
| if (System.getProperty("os.name").contains("Windows") || | ||
| (System.getProperty("os.name").contains("OS X") | ||
| && System.getProperty("os.arch").contains("x86_64"))) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
qq, this is a dependency brought in by grpc-s2a, right?
Just wanted to confirm as I don't think this is brought in by Gax.
There was a problem hiding this comment.
I also feel this is some low level implementation details that Gax does not need to be aware of, can this logic be handled in grpc-java instead of Gax?
There was a problem hiding this comment.
Hi @lqiu96 and @blakeli0 apologies for the delayed response.
qq, this is a dependency brought in by grpc-s2a, right?Just wanted to confirm as I don't think this is brought in by Gax.
Yes, this is a dependency of grpc-s2a. This dependency is dropping support on the specified platforms so we don't want to try and use S2A when we are on those platforms.
can this logic be handled in grpc-java instead of Gax
This is a good point, technically, I believe the answer is yes, we could just return null / throw an error if you try and use S2A on these platforms. However, there are a few more things I need to confirm, I will follow up with you both and grpc-java internally after I investigate and confirm a few things, and then we can proceed with this PR accordingly.
lqiu96
left a comment
There was a problem hiding this comment.
Can you also update the title to use conventional commits
Done. |
S2A has a runtime dependency on netty-tcnative.
netty-tcnative is looking to drop support for windows and macos intel based builds. To avoid runtime errors for users of the Cloud SDK, disable S2A if they are running on these platforms.