[SPARK-43977][CONNECT] Fix unexpected check result of dev/connect-jvm-client-mima-check
#41473
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LuciferYang
changed the title
protobuf/package to protobuf/assembly and add a line break after error messageprotobuf/package to protobuf/assembly and add a line break after error message
LuciferYang
changed the title
protobuf/package to protobuf/assembly and add a line break after error messageprotobuf/package to protobuf/assembly and add a line break after write error message
LuciferYang
changed the title
protobuf/package to protobuf/assembly and add a line break after write error messagedev/connect-jvm-client-mima-check
| @@ -95,7 +95,7 @@ object CheckConnectJvmClientCompatibility { | |||
| } catch { | |||
| case e: Throwable => | |||
| println(e.getMessage) | |||
| resultWriter.write(s"ERROR: ${e.getMessage}") | |||
| resultWriter.write(s"ERROR: ${e.getMessage}\n") | |||
There was a problem hiding this comment.
The old code seems to exist since Apache Spark 3.4.0 but SPARK-43977 is filed for 3.5.0 only. Do we need to backport this?
There was a problem hiding this comment.
@dongjoon-hyun Thanks for your review. Good question. Due to Spark 3.4.x will not add new modules for mima check with connect module, this issue should not be triggered, so I think just fixing it in Spark 3.5.0 is enough now
dongjoon-hyun
approved these changes
Jun 6, 2023
|
Merged to master. Thanks @dongjoon-hyun |
czxm
pushed a commit
to czxm/spark
that referenced
this pull request
Jun 12, 2023
…m-client-mima-check` ### What changes were proposed in this pull request? There is an unexpected check scenario of `dev/connect-jvm-client-mima-check` as follows: 1. run `build/sbt "protobuf/clean"` 2. run `dev/connect-jvm-client-mima-check` Then is a strange check result ``` Failed to find the jar: spark-protobuf-assembly(.*).jar or spark-protobuf(.*)3.5.0-SNAPSHOT.jar inside folder: /Users/yangjie01/SourceCode/git/spark-mine-sbt/connector/protobuf/target. This file can be generated by similar to the following command: build/sbt package|assembly finish connect-client-jvm module mima check ... connect-client-jvm module mima check passed. ``` There are both error messages and checks successful as above. So this pr do the following changes to fix this unexpected check scenario: 1. Add a line break after write error message in `CheckConnectJvmClientCompatibility` to avoid `RESULT_SIZE` being considered as `0`: after this change, run the above commands, the check result result is expected: ``` Do connect-client-jvm module mima check ... Failed to find the jar: spark-protobuf-assembly(.*).jar or spark-protobuf(.*)3.5.0-SNAPSHOT.jar inside folder: /Users/yangjie01/SourceCode/git/spark-mine-sbt/connector/protobuf/target. This file can be generated by similar to the following command: build/sbt package|assembly finish connect-client-jvm module mima check ... ERROR: Failed to find the jar: spark-protobuf-assembly(.*).jar or spark-protobuf(.*)3.5.0-SNAPSHOT.jar inside folder: /Users/yangjie01/SourceCode/git/spark-mine-sbt/connector/protobuf/target. This file can be generated by similar to the following command: build/sbt package|assembly connect-client-jvm module mima check failed. ``` 2. Change `protobuf/package` to `protobuf/assembly` to ensure spark-protobuf uber jar always present after this change, run the above commands, the check result result is successful: ``` Do connect-client-jvm module mima check ... finish connect-client-jvm module mima check ... connect-client-jvm module mima check passed. ``` ### Why are the changes needed? Fix wrong check result of `dev/connect-jvm-client-mima-check` ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? - Pass GitHub Actions - Do manual checks as pr description Closes apache#41473 from LuciferYang/SPARK-43590-FOLLOWUP. Authored-by: yangjie01 <yangjie01@baidu.com> Signed-off-by: yangjie01 <yangjie01@baidu.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
There is an unexpected check scenario of
dev/connect-jvm-client-mima-checkas follows:build/sbt "protobuf/clean"dev/connect-jvm-client-mima-checkThen is a strange check result
There are both error messages and checks successful as above.
So this pr do the following changes to fix this unexpected check scenario:
CheckConnectJvmClientCompatibilityto avoidRESULT_SIZEbeing considered as0:after this change, run the above commands, the check result result is expected:
protobuf/packagetoprotobuf/assemblyto ensure spark-protobuf uber jar always presentafter this change, run the above commands, the check result result is successful:
Why are the changes needed?
Fix wrong check result of
dev/connect-jvm-client-mima-checkDoes this PR introduce any user-facing change?
No
How was this patch tested?