-
Notifications
You must be signed in to change notification settings - Fork 3k
Build: Test flink modules for both scala 2.11 and 2.12 #4179
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: Test flink modules for both scala 2.11 and 2.12 #4179
Conversation
.github/workflows/flink-ci.yml
Outdated
| matrix: | ||
| jvm: [8, 11] | ||
| flink: ['1.12', '1.13', '1.14'] | ||
| scala: ['2.11', '2.12'] |
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.
This makes Flink testing very expensive. Is it possible to just test the latest Flink with all versions of Scala and test the old ones with just 2.12?
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.
+1. This will add quite a lot to the Flink CI time.
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.
Sounds reasonable ! Just updated the patch.
| key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle') }} | ||
| restore-keys: ${{ runner.os }}-gradle | ||
| - run: echo -e "$(ip addr show eth0 | grep "inet\b" | awk '{print $2}' | cut -d/ -f1)\t$(hostname -f) $(hostname -s)" | sudo tee -a /etc/hosts | ||
| - run: ./gradlew -DsparkVersions= -DhiveVersions= -DflinkVersions=${{ matrix.flink }} :iceberg-flink:iceberg-flink-${{ matrix.flink }}:check :iceberg-flink:iceberg-flink-runtime-${{ matrix.flink }}:check -DscalaVersion=2.11 -DknownScalaVersions=2.11 -Pquick=true -x javadoc |
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.
|
@rdblue , Is there any other concern for this PR ? I will suggest to merge this one in a little high priority because it is adding extra Scala 2.11 build & validation for all further PRs :-) |
.github/workflows/flink-ci.yml
Outdated
| restore-keys: ${{ runner.os }}-gradle | ||
| - run: echo -e "$(ip addr show eth0 | grep "inet\b" | awk '{print $2}' | cut -d/ -f1)\t$(hostname -f) $(hostname -s)" | sudo tee -a /etc/hosts | ||
| - run: ./gradlew -DsparkVersions= -DhiveVersions= -DflinkVersions=${{ matrix.flink }} :iceberg-flink:iceberg-flink-${{ matrix.flink }}:check :iceberg-flink:iceberg-flink-runtime-${{ matrix.flink }}:check -Pquick=true -x javadoc | ||
| - run: ./gradlew -DsparkVersions= -DhiveVersions= -DflinkVersions=${{ matrix.flink }} :iceberg-flink:iceberg-flink-${{ matrix.flink }}:check :iceberg-flink:iceberg-flink-runtime-${{ matrix.flink }}:check -DscalaVersion=2.12 -DknownScalaVersions=2.12 -Pquick=true -x javadoc |
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.
knownScalaVersions shouldn't need to change, right? That is the list of versions that are supported by the build.
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.
Sounds reasonable to me, I've just reverted this change.
|
@openinx, this looks good to me other than setting |
|
Everything looks good now. I just got this PR merged, thanks all for reviewing ! |
Run flink travis CI for both scala 2.11 & 2.12