-
Notifications
You must be signed in to change notification settings - Fork 39
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
build(fix): bump-up protobufVersion to 3.21.12 (latest) #760
Conversation
@laysakura Can you please also commit the diff of the protocol buffers generated classes? |
@laysakura I'm wondering if this change is backward compatible. Did you test it or find any document for it? |
@brfrn169 Not yet. I would like to ask you whether these kind of Since:
I'm not sure whether these diffs are usual one or not. If they usually occur when updating protobufVersion, I will investigate the backward compatibility. |
@laysakura These kind of
So maybe it's faster to test the compatibility from my side? |
@brfrn169 Got it. Thanks.
That's true but I would also like to cultivate how to test the proto compatibility in this project.
|
@laysakura Sure. Let me test it. And I'll show you the result and the procedure. Thanks. |
@laysakura Sorry for the late response. I tested the backward compatibility of this change, and it was successful. In the test, I started ScalarDB Servers with the change (the The procedure to test the backward compatibility is as follows:
git switch build/bumpup-protoc-version2
./gradlew :server:docker
scalar.db.contact_points=jdbc:mysql://mysql:3306/
scalar.db.username=root
scalar.db.password=mysql
scalar.db.storage=jdbc
scalar.db.consensus_commit.async_commit.enabled=false
scalar.db.server.port=60051
scalar.db.contact_points=jdbc:mysql://mysql:3306/
scalar.db.username=root
scalar.db.password=mysql
scalar.db.storage=jdbc
scalar.db.consensus_commit.async_commit.enabled=false
scalar.db.server.port=60052
docker network create scalardb-server-network
docker run --name mysql --network scalardb-server-network -e MYSQL_ROOT_PASSWORD=mysql -d -p 3306:3306 mysql
docker run --name scalardb-server1 --network scalardb-server-network -v ${PWD}/database1.properties:/scalardb/server/database.properties.tmpl -d -p 60051:60051 ghcr.io/scalar-labs/scalardb-server:4.0.0-SNAPSHOT
docker run --name scalardb-server2 --network scalardb-server-network -v ${PWD}/database2.properties:/scalardb/server/database.properties.tmpl -d -p 60052:60052 ghcr.io/scalar-labs/scalardb-server:4.0.0-SNAPSHOT
git switch master
./gradlew :server:integrationTestScalarDbServer -Dscalardb.server.external_server_used=true |
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.
LGTM! Thank you!
@brfrn169 Thank you for your review. I'll merge this PR after the CI pass again. I also learned how to test the server's backward compatibility. |
@laysakura Yes, that's a good idea. You meant to add something a Github actions workflow to test the backward compatibility, right? |
@laysakura Thank you! |
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.
LGTM, thank you!
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.
LGTM, thanks!
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.
LGTM! Thank you!
Problem
The following command fails in M1 Mac (osx-aarch_64).
./gradlew :rpc:generateProto # called from `./gradlew test`, for example
This is because the version 3.20.3 does not include
protoc
binary forosx-aarch_64
in the maven repository, while 3.21.12 does.Refs: protocolbuffers/protobuf#8062
What I did
Bump up
protobufVersion
from 3.20.3 to 3.21.12 (latest).I'm not sure...
The command:
./gradlew :rpc:generateProto
leads to the following diffs. Currently I don't commit the diffs but I will if necessary.
rpc-generateProto.diff.txt