Copilot commented on code in PR #17945: URL: https://github.com/apache/iotdb/pull/17945#discussion_r3449658061
########## iotdb-client/service-rpc/src/main/java/org/apache/iotdb/rpc/TNonblockingTransportWrapper.java: ########## @@ -27,8 +27,8 @@ import java.nio.channels.SocketChannel; /** - * In Thrift 0.14.1, TNonblockingSocket's constructor throws a never-happened exception. So, we - * screen the exception https://issues.apache.org/jira/browse/THRIFT-5412 + * TNonblockingSocket's constructor throws a never-happened exception. So, we screen the exception + * https://issues.apache.org/jira/browse/THRIFT-5412 */ Review Comment: The updated Javadoc has awkward phrasing (“never-happened exception”, “screen the exception”), which makes the intent hard to understand. Consider rewording now that the comment was touched. ########## .github/workflows/client-cpp-package.yml: ########## @@ -46,18 +46,32 @@ jobs: steps: - uses: actions/checkout@v5 if: github.event_name == 'push' && startsWith(github.ref, 'refs/heads/rc/') - - uses: dorny/paths-filter@v3 + with: + fetch-depth: 0 + - name: Detect C++ package changes id: filter if: github.event_name == 'push' && startsWith(github.ref, 'refs/heads/rc/') - with: - filters: | - cpp: - - 'iotdb-client/client-cpp/**' - - 'iotdb-client/pom.xml' - - 'iotdb-protocol/thrift-datanode/src/main/thrift/client.thrift' - - 'iotdb-protocol/thrift-commons/src/main/thrift/common.thrift' - - '.github/workflows/client-cpp-package.yml' - - '.github/scripts/package-client-cpp-*.sh' + shell: bash + run: | + set -euo pipefail + + BASE="${{ github.event.before }}" + HEAD="${{ github.sha }}" + if [[ "$BASE" =~ ^0+$ ]]; then + BASE="$(git hash-object -t tree /dev/null)" + fi + + cpp=false + while IFS= read -r file; do + case "$file" in + iotdb-client/client-cpp/*|iotdb-client/pom.xml|iotdb-protocol/thrift-datanode/src/main/thrift/client.thrift|iotdb-protocol/thrift-commons/src/main/thrift/common.thrift|.github/workflows/client-cpp-package.yml|.github/scripts/package-client-cpp-*.sh) + cpp=true + break + ;; + esac + done < <(git diff --name-only "$BASE" "$HEAD") Review Comment: The C++ change detector only matches one path segment under `iotdb-client/client-cpp/*`, so changes in nested paths (e.g., `iotdb-client/client-cpp/src/...`) may not trigger packaging on `rc/**` branches. ########## .github/workflows/multi-language-client.yml: ########## @@ -50,36 +50,55 @@ jobs: go: ${{ steps.filter.outputs.go }} steps: - uses: actions/checkout@v5 - - uses: dorny/paths-filter@v3 - id: filter with: - filters: | - cpp: - - 'iotdb-client/pom.xml' - - 'iotdb-client/client-cpp/**' - - 'iotdb-protocol/thrift-datanode/src/main/thrift/client.thrift' - - 'iotdb-protocol/thrift-commons/src/main/thrift/common.thrift' - - '.github/workflows/multi-language-client.yml' - - '.github/workflows/client-cpp-package.yml' - - '.github/scripts/package-client-cpp-*.sh' - python: - - 'pom.xml' - - 'iotdb-client/pom.xml' - - 'iotdb-client/client-py/**' - - 'docker/src/main/Dockerfile-1c1d' - - '.github/workflows/multi-language-client.yml' - go: - - 'pom.xml' - - 'iotdb-core/**' - - 'iotdb-api/**' - - 'iotdb-protocol/**' - - 'distribution/**' - - 'integration-test/**' - - 'iotdb-client/session/**' - - 'iotdb-client/jdbc/**' - - 'iotdb-client/cli/**' - - 'iotdb-client/service-rpc/**' - - '.github/workflows/multi-language-client.yml' + fetch-depth: 0 + - name: Detect changed client paths + id: filter + shell: bash + run: | + set -euo pipefail + + if [[ "${{ github.event_name }}" == "pull_request" ]]; then + BASE="${{ github.event.pull_request.base.sha }}" + HEAD="${{ github.sha }}" + elif [[ "${{ github.event_name }}" == "push" ]]; then + BASE="${{ github.event.before }}" + HEAD="${{ github.sha }}" + else + echo "cpp=true" >> "$GITHUB_OUTPUT" + echo "python=true" >> "$GITHUB_OUTPUT" + echo "go=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + + if [[ "$BASE" =~ ^0+$ ]]; then + BASE="$(git hash-object -t tree /dev/null)" + fi + + cpp=false + python=false + go=false + while IFS= read -r file; do + case "$file" in + iotdb-client/pom.xml|iotdb-client/client-cpp/*|iotdb-protocol/thrift-datanode/src/main/thrift/client.thrift|iotdb-protocol/thrift-commons/src/main/thrift/common.thrift|.github/workflows/multi-language-client.yml|.github/workflows/client-cpp-package.yml|.github/scripts/package-client-cpp-*.sh) + cpp=true + ;; + esac + case "$file" in + pom.xml|iotdb-client/pom.xml|iotdb-client/client-py/*|docker/src/main/Dockerfile-1c1d|.github/workflows/multi-language-client.yml) + python=true + ;; + esac + case "$file" in + pom.xml|iotdb-core/*|iotdb-api/*|iotdb-protocol/*|distribution/*|integration-test/*|iotdb-client/session/*|iotdb-client/jdbc/*|iotdb-client/cli/*|iotdb-client/service-rpc/*|.github/workflows/multi-language-client.yml) + go=true + ;; + esac + done < <(git diff --name-only "$BASE" "$HEAD") Review Comment: The changed-path detection only matches one path segment under directories (e.g., `iotdb-client/client-cpp/*`, `iotdb-core/*`). In bash pattern matching, `*` won’t reliably match nested paths like `iotdb-client/client-cpp/src/...`, which can cause language jobs to be skipped even when relevant files changed. ########## iotdb-client/service-rpc/src/main/java/org/apache/iotdb/rpc/TSocketWrapper.java: ########## @@ -24,8 +24,8 @@ import org.apache.thrift.transport.TTransportException; /** - * In Thrift 0.14.1, TSocket's constructor throws a never-happened exception. So, we screen the - * exception https://issues.apache.org/jira/browse/THRIFT-5412 + * TSocket's constructor throws a never-happened exception. So, we screen the exception + * https://issues.apache.org/jira/browse/THRIFT-5412 */ Review Comment: The updated Javadoc has awkward phrasing (“never-happened exception”, “screen the exception”), which makes the intent hard to understand. Consider rewording now that the comment was touched. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
