Skip to content

Commit

Permalink
[router] Alpini dead code removal (#1394)
Browse files Browse the repository at this point in the history
Besides dead code removal, also moved a few functions from Alpini's
CollectionUtil to Venice's CollectionUtils.

Deleted the following entire modules:

alpini-common-cli
alpini-common-const
alpini-common-io
alpini-common-log
alpini-common-native
alpini-common-test

Also removed the Alpini Functional Tests CI task, since there is no such 
test left, and made the Alpini Unit Tests run on every commit, rather 
than only when these files are modified, since they run pretty quickly 
now.
  • Loading branch information
FelixGV authored Dec 14, 2024
1 parent 2af0fa9 commit e27379d
Show file tree
Hide file tree
Showing 242 changed files with 210 additions and 52,025 deletions.
93 changes: 4 additions & 89 deletions .github/workflows/VeniceCI-CompatibilityTests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,27 +55,16 @@ jobs:
jdk: [8, 11, 17]
runs-on: ubuntu-latest
timeout-minutes: 120
outputs:
alpini_touched: ${{ steps.check_alpini_files_changed.outputs.alpini }}
steps:
- uses: actions/checkout@v4
with:
# Checkout as many commits as needed for the diff
fetch-depth: 2
- name: Check if files have changed
uses: dorny/paths-filter@v3
id: check_alpini_files_changed
with:
filters: |
alpini:
- 'internal/alpini/**'
- uses: actions/checkout@v4
if: steps.check_alpini_files_changed.outputs.alpini == 'true'
with:
fetch-depth: 0
- name: Set up JDK
uses: actions/setup-java@v4
if: steps.check_alpini_files_changed.outputs.alpini == 'true'
with:
java-version: ${{ matrix.jdk }}
distribution: 'temurin'
Expand All @@ -92,101 +81,27 @@ jobs:
# echo "java.security file after modifications: "
# cat "$JAVA_HOME/conf/security/java.security"
- shell: bash
if: steps.check_alpini_files_changed.outputs.alpini == 'true'
run: |
git remote set-head origin --auto
git remote add upstream https://github.com/linkedin/venice
git fetch upstream
- name: Setup Gradle
if: steps.check_alpini_files_changed.outputs.alpini == 'true'
uses: gradle/actions/setup-gradle@v4
- name: Run alpini unit tests
if: steps.check_alpini_files_changed.outputs.alpini == 'true'
run: ./gradlew --continue --no-daemon -DmaxParallelForks=1 alpiniUnitTest
- name: Package Build Artifacts
if: steps.check_alpini_files_changed.outputs.alpini == 'true' && (success() || failure())
if: (success() || failure())
shell: bash
run: |
mkdir ${{ github.job }}-artifacts
find . -path "**/build/reports/*" -or -path "**/build/test-results/*" > artifacts.list
rsync -R --files-from=artifacts.list . ${{ github.job }}-artifacts
tar -zcvf ${{ github.job }}-jdk${{ matrix.jdk }}-logs.tar.gz ${{ github.job }}-artifacts
- name: Upload Build Artifacts
if: steps.check_alpini_files_changed.outputs.alpini == 'true' && (success() || failure())
if: (success() || failure())
uses: actions/upload-artifact@v4
with:
name: ${{ github.job }}
path: ${{ github.job }}-jdk${{ matrix.jdk }}-logs.tar.gz
retention-days: 30

AlpiniFunctionalTests:
strategy:
fail-fast: false
matrix:
jdk: [17]
runs-on: ubuntu-latest
timeout-minutes: 120
outputs:
alpini_touched: ${{ steps.check_alpini_files_changed.outputs.alpini }}
steps:
- uses: actions/checkout@v4
with:
# Checkout as many commits as needed for the diff
fetch-depth: 2
- name: Check if files have changed
uses: dorny/paths-filter@v3
id: check_alpini_files_changed
with:
filters: |
alpini:
- 'internal/alpini/**'
- uses: actions/checkout@v4
if: steps.check_alpini_files_changed.outputs.alpini == 'true'
with:
fetch-depth: 0
- name: Set up JDK
uses: actions/setup-java@v4
if: steps.check_alpini_files_changed.outputs.alpini == 'true'
with:
java-version: ${{ matrix.jdk }}
distribution: 'temurin'
cache: 'gradle'
# - name: Allow Deprecated TLS versions for Alpini tests
# run: |
# echo "java.security file before modifications: "
# cat "$JAVA_HOME/conf/security/java.security"

# # This is possibly flaky but
# sed -i 's/TLSv1, //g' "$JAVA_HOME/conf/security/java.security" # Allow TLSv1
# sed -i 's/TLSv1.1, //g' "$JAVA_HOME/conf/security/java.security" # Allow TLSv1.1

# echo "java.security file after modifications: "
# cat "$JAVA_HOME/conf/security/java.security"
- shell: bash
if: steps.check_alpini_files_changed.outputs.alpini == 'true'
run: |
git remote set-head origin --auto
git remote add upstream https://github.com/linkedin/venice
git fetch upstream
- name: Setup Gradle
if: steps.check_alpini_files_changed.outputs.alpini == 'true'
uses: gradle/actions/setup-gradle@v4
- name: Run alpini functional tests
if: steps.check_alpini_files_changed.outputs.alpini == 'true'
run: ./gradlew --continue --no-daemon -DmaxParallelForks=1 alpiniFunctionalTest
- name: Package Build Artifacts
if: steps.check_alpini_files_changed.outputs.alpini == 'true' && (success() || failure())
shell: bash
run: |
mkdir ${{ github.job }}-artifacts
find . -path "**/build/reports/*" -or -path "**/build/test-results/*" > artifacts.list
rsync -R --files-from=artifacts.list . ${{ github.job }}-artifacts
tar -zcvf ${{ github.job }}-jdk${{ matrix.jdk }}-logs.tar.gz ${{ github.job }}-artifacts
- name: Upload Build Artifacts
if: steps.check_alpini_files_changed.outputs.alpini == 'true' && (success() || failure())
uses: actions/upload-artifact@v4
with:
name: ${{ github.job }}
name: ${{ github.job }}-jdk${{ matrix.jdk }}
path: ${{ github.job }}-jdk${{ matrix.jdk }}-logs.tar.gz
retention-days: 30

Expand Down Expand Up @@ -257,7 +172,7 @@ jobs:
strategy:
fail-fast: false
runs-on: ubuntu-latest
needs: [AvroCompatibilityTests, AlpiniUnitTests, AlpiniFunctionalTests, PulsarVeniceIntegrationTests]
needs: [AvroCompatibilityTests, AlpiniUnitTests, PulsarVeniceIntegrationTests]
timeout-minutes: 120
steps:
- name: AllIsWell
Expand Down
14 changes: 1 addition & 13 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,6 @@ subprojects {
def ALPINI_TEST_FILTER = 'com.linkedin.alpini.*'
def ALPINI_NETTY_TEST_FILTER = 'io.netty.*'
def ALPINI_UNIT_TEST_TASK_NAME = 'alpiniUnitTest'
def ALPINI_FUNCTIONAL_TEST_TASK_NAME = 'alpiniFunctionalTest'

tasks.withType(Test) {
mustRunAfter tasks.withType(SpotBugsTask)
Expand Down Expand Up @@ -419,7 +418,7 @@ subprojects {
println "jvmArgs=$allJvmArgs"
}

if (name != ALPINI_UNIT_TEST_TASK_NAME && name != ALPINI_FUNCTIONAL_TEST_TASK_NAME) {
if (name != ALPINI_UNIT_TEST_TASK_NAME) {
filter {
excludeTestsMatching ALPINI_TEST_FILTER
excludeTestsMatching ALPINI_NETTY_TEST_FILTER
Expand Down Expand Up @@ -564,17 +563,6 @@ subprojects {
}
}

task "$ALPINI_FUNCTIONAL_TEST_TASK_NAME"(type: Test) {
useTestNG() {
includeGroups 'functional'
}
filter {
includeTestsMatching ALPINI_TEST_FILTER
includeTestsMatching ALPINI_NETTY_TEST_FILTER
failOnNoMatchingTests = false
}
}

tasks.withType(Jar) {
zip64 = true
duplicatesStrategy = DuplicatesStrategy.FAIL
Expand Down
6 changes: 6 additions & 0 deletions gradle/spotbugs/exclude.xml
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,12 @@
</Class>
</Or>
</Match>
<Match>
<Bug pattern="RC_REF_COMPARISON"/>
<Or>
<Class name="com.linkedin.venice.utils.CollectionUtilsTest"/>
</Or>
</Match>
<Match>
<!--Auto-generated classes. No plan to fix-->
<Bug pattern="RI_REDUNDANT_INTERFACES"/>
Expand Down
1 change: 0 additions & 1 deletion internal/alpini/common/alpini-common-base/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ configurations {

dependencies {
testImplementation libraries.testng
implementation project(':internal:alpini:common:alpini-common-const')
implementation libraries.jacksonDatabind
implementation libraries.jsr305
implementation libraries.log4j2api
Expand Down
Loading

0 comments on commit e27379d

Please sign in to comment.