remove redundant scripts for BOM availability and EAP version extraction #167
Conversation
WalkthroughThis change modifies the EAP Gradle build triggering mechanism by replacing BOM availability verification with a streamlined Maven metadata lookup that extracts the latest Ktor EAP version, eliminating retries, HTTP status checks, and fallback logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.teamcity/src/subprojects/train/TriggerProjectSamplesOnEAP.kt (2)
418-423: Consider adding retry logic for network resilience.The
createEAPGradleInitScript()function (line 36) uses--retry 2 --retry-delay 2for fetching the plugin version, which provides resilience against transient network failures. The version resolver script could benefit from similar retry logic to ensure reliable EAP version resolution.Apply this diff to add retry logic:
# Download the metadata file - if ! curl -s "${'$'}METADATA_URL" -o "${'$'}TEMP_FILE"; then + if ! curl -fsSL --connect-timeout 5 --max-time 15 --retry 2 --retry-delay 2 "${'$'}METADATA_URL" -o "${'$'}TEMP_FILE"; then echo "Failed to download metadata from ${'$'}METADATA_URL" rm -f "${'$'}TEMP_FILE" exit 1 fi
416-416: Ensure temp file cleanup on early exit.With
set -eat line 409, the script will exit immediately on any error. If an error occurs between creating the temp file (line 416) and the cleanup (line 430), the temp file will not be removed, potentially accumulating over time.Use a
trapto guarantee cleanup:# Create a temporary file for the metadata TEMP_FILE=$(mktemp) + trap 'rm -f "$TEMP_FILE"' EXIT # Download the metadata file if ! curl -s "${'$'}METADATA_URL" -o "${'$'}TEMP_FILE"; then echo "Failed to download metadata from ${'$'}METADATA_URL" - rm -f "${'$'}TEMP_FILE" exit 1 fi # Extract the latest version using grep and sed # This pattern looks for a <latest>version</latest> tag LATEST_VERSION=$(grep -o '<latest>[^<]*</latest>' "${'$'}TEMP_FILE" | sed 's/<latest>\(.*\)<\/latest>/\1/') - # Clean up temp file - rm -f "${'$'}TEMP_FILE" - if [ -z "${'$'}LATEST_VERSION" ]; thenAlso applies to: 430-430
| # Extract the latest version using grep and sed | ||
| # This pattern looks for a <latest>version</latest> tag | ||
| LATEST_VERSION=$(grep -o '<latest>[^<]*</latest>' "${'$'}TEMP_FILE" | sed 's/<latest>\(.*\)<\/latest>/\1/') |
There was a problem hiding this comment.
XML parsing with grep/sed is fragile and may fail on formatted XML.
The pattern '<latest>[^<]*</latest>' assumes the XML tag is on a single line. Maven metadata XML is often formatted with newlines and indentation, which will cause this grep pattern to fail. This could break the version resolution for EAP builds.
Consider using a more robust approach:
- # Extract the latest version using grep and sed
- # This pattern looks for a <latest>version</latest> tag
- LATEST_VERSION=$(grep -o '<latest>[^<]*</latest>' "${'$'}TEMP_FILE" | sed 's/<latest>\(.*\)<\/latest>/\1/')
+ # Extract the latest version using sed with multiline support
+ # This handles XML formatted across multiple lines
+ LATEST_VERSION=$(sed -n 's/.*<latest>\(.*\)<\/latest>.*/\1/p' "${'$'}TEMP_FILE" | tr -d '[:space:]')Or for more robustness, use xmllint if available:
- # Extract the latest version using grep and sed
- # This pattern looks for a <latest>version</latest> tag
- LATEST_VERSION=$(grep -o '<latest>[^<]*</latest>' "${'$'}TEMP_FILE" | sed 's/<latest>\(.*\)<\/latest>/\1/')
+ # Extract the latest version using xmllint or sed fallback
+ if command -v xmllint &> /dev/null; then
+ LATEST_VERSION=$(xmllint --xpath "string(//latest)" "${'$'}TEMP_FILE")
+ else
+ # Fallback to sed that handles multiline XML
+ LATEST_VERSION=$(sed -n 's/.*<latest>\([^<]*\)<\/latest>.*/\1/p' "${'$'}TEMP_FILE" | tr -d '[:space:]' | head -n 1)
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Extract the latest version using grep and sed | |
| # This pattern looks for a <latest>version</latest> tag | |
| LATEST_VERSION=$(grep -o '<latest>[^<]*</latest>' "${'$'}TEMP_FILE" | sed 's/<latest>\(.*\)<\/latest>/\1/') | |
| # Extract the latest version using sed with multiline support | |
| # This handles XML formatted across multiple lines | |
| LATEST_VERSION=$(sed -n 's/.*<latest>\(.*\)<\/latest>.*/\1/p' "${'$'}TEMP_FILE" | tr -d '[:space:]') |
| # Extract the latest version using grep and sed | |
| # This pattern looks for a <latest>version</latest> tag | |
| LATEST_VERSION=$(grep -o '<latest>[^<]*</latest>' "${'$'}TEMP_FILE" | sed 's/<latest>\(.*\)<\/latest>/\1/') | |
| # Extract the latest version using xmllint or sed fallback | |
| if command -v xmllint &> /dev/null; then | |
| LATEST_VERSION=$(xmllint --xpath "string(//latest)" "${'$'}TEMP_FILE") | |
| else | |
| # Fallback to sed that handles multiline XML | |
| LATEST_VERSION=$(sed -n 's/.*<latest>\([^<]*\)<\/latest>.*/\1/p' "${'$'}TEMP_FILE" | tr -d '[:space:]' | head -n 1) | |
| fi |
No description provided.