carverauto/serviceradar

Fork 0

Updates/bazel fixes #2440

Merged

mfreeman451 merged 2 commits from refs/pull/2440/head into main

2025-11-20 21:28:46 +00:00

mfreeman451 commented

2025-11-20 21:28:36 +00:00

(Migrated from github.com)

Owner

Imported from GitHub pull request.

Original GitHub pull request: #1972
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1972
Original created: 2025-11-20T21:28:36Z
Original updated: 2025-11-20T21:29:50Z
Original head: carverauto/serviceradar:updates/bazel_fixes
Original base: main
Original merged: 2025-11-20T21:28:46Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

I have signed the DCO?
The build completes without errors?
All tests are passing when running make test?

PR Type

Enhancement, Bug fix

Description

Add fallback to local execution when remote RBE fails
Mark RBE platform as RPM-incompatible to prevent toolchain issues
Improve release workflow robustness with graceful degradation

Diagram Walkthrough

flowchart LR
  A["Remote RBE execution"] -->|Success| B["Publish packages"]
  A -->|Failure| C["Fallback to local execution"]
  C --> B
  D["RBE platform config"] -->|Add constraint| E["Mark RPM-incompatible"]

File Walkthrough

Relevant files

Enhancement

release.yml

Add fallback to local execution on remote failure

.github/workflows/release.yml

Wrap remote bazel run in conditional with fallback logic
Execute locally without remote executor/cache on remote failure
Disable remote cache acceptance and upload on fallback
Set local job parallelism to 8 for fallback execution

+14/-1

Bug fix

BUILD

Mark RBE platform as RPM-incompatible

build/rbe/BUILD

Add @rules_pkg//pkg:not_compatible constraint to RBE platform
Prevents incompatible toolchain binaries from being used on executor

+1/-0

Imported from GitHub pull request. Original GitHub pull request: #1972 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1972 Original created: 2025-11-20T21:28:36Z Original updated: 2025-11-20T21:29:50Z Original head: carverauto/serviceradar:updates/bazel_fixes Original base: main Original merged: 2025-11-20T21:28:46Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Enhancement, Bug fix ___ ### **Description** - Add fallback to local execution when remote RBE fails - Mark RBE platform as RPM-incompatible to prevent toolchain issues - Improve release workflow robustness with graceful degradation ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Remote RBE execution"] -->|Success| B["Publish packages"] A -->|Failure| C["Fallback to local execution"] C --> B D["RBE platform config"] -->|Add constraint| E["Mark RPM-incompatible"] ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>release.yml</strong><dd><code>Add fallback to local execution on remote failure</code>                </dd></summary> <hr> .github/workflows/release.yml <ul><li>Wrap remote bazel run in conditional with fallback logic<br> <li> Execute locally without remote executor/cache on remote failure<br> <li> Disable remote cache acceptance and upload on fallback<br> <li> Set local job parallelism to 8 for fallback execution</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1972/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34">+14/-1</a>    </td> </tr> </table></td></tr><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>BUILD</strong><dd><code>Mark RBE platform as RPM-incompatible</code>                                        </dd></summary> <hr> build/rbe/BUILD <ul><li>Add <code>@rules_pkg//pkg:not_compatible</code> constraint to RBE platform<br> <li> Prevents incompatible toolchain binaries from being used on executor</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1972/files#diff-ac0d5204dc44afc495e4f6899ae668175ade69fa6b3324c94261ae2461586a21">+1/-0</a>      </td> </tr> </table></td></tr></tr></tbody></table> </details> ___

qodo-code-review[bot] commented

2025-11-20 21:28:59 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1972#issuecomment-3560117428
Original created: 2025-11-20T21:28:59Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Action Logging: The workflow adds fallback execution but does not log structured audit details (actor, outcome, timestamps) for the critical release publishing action beyond a generic stderr message. Referred Code # Prefer remote RBE for speed, but fall back to local execution (no remote cache/exec) # when toolchain binaries (e.g., copy_directory) are unavailable on the executor. if ! bazel run --config=remote --stamp //release:publish_packages -- "${args[@]}"; then echo "Remote bazel run failed; retrying locally without remote executor/cache" >&2 bazel run \ --remote_executor= \ --remote_cache= \ --noremote_accept_cached \ --noremote_upload_local_results \ --jobs=8 \ --stamp \ //release:publish_packages \ -- "${args[@]}" fi Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Unstructured Logs: The fallback path logs a free-form message to stderr without structured logging, making automated parsing and monitoring difficult. Referred Code `echo "Remote bazel run failed; retrying locally without remote executor/cache" >&2 bazel run \` Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1972#issuecomment-3560117428 Original created: 2025-11-20T21:28:59Z --- ## PR Compliance Guide 🔍  Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary>🎫 <strong>No ticket provided </strong></summary> - [ ] Create ticket/issue  </details></td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=4>🟢</td><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=2>⚪</td> <td><details> <summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/1972/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R238-R251'><strong>Action Logging</strong></a>: The workflow adds fallback execution but does not log structured audit details (actor, <br>outcome, timestamps) for the critical release publishing action beyond a generic stderr <br>message.<br> <details open><summary>Referred Code</summary> ```yaml # Prefer remote RBE for speed, but fall back to local execution (no remote cache/exec) # when toolchain binaries (e.g., copy_directory) are unavailable on the executor. if ! bazel run --config=remote --stamp //release:publish_packages -- "${args[@]}"; then echo "Remote bazel run failed; retrying locally without remote executor/cache" >&2 bazel run \ --remote_executor= \ --remote_cache= \ --noremote_accept_cached \ --noremote_upload_local_results \ --jobs=8 \ --stamp \ //release:publish_packages \ -- "${args[@]}" fi ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary><strong>Generic: Secure Logging Practices</strong></summary><br> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/1972/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R241-R241'><strong>Unstructured Logs</strong></a>: The fallback path logs a free-form message to stderr without structured logging, making <br>automated parsing and monitoring difficult.<br> <details open><summary>Referred Code</summary> ```yaml echo "Remote bazel run failed; retrying locally without remote executor/cache" >&2 bazel run \ ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2">   </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>

qodo-code-review[bot] commented

2025-11-20 21:29:50 +00:00

(Migrated from github.com)

Author

Owner

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1972#issuecomment-3560119709
Original created: 2025-11-20T21:29:50Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Rely on Bazel's execution strategy The suggestion proposes removing the shell-level retry script from the GitHub workflow. It argues that the added Bazel platform constraint is the correct and more efficient method for handling toolchain incompatibilities, rendering the all-or-nothing fallback redundant. Examples: .github/workflows/release.yml [240-251] `if ! bazel run --config=remote --stamp //release:publish_packages -- "${args[@]}"; then echo "Remote bazel run failed; retrying locally without remote executor/cache" >&2 bazel run \ --remote_executor= \ --remote_cache= \ --noremote_accept_cached \ --noremote_upload_local_results \ --jobs=8 \ --stamp \ //release:publish_packages \ ... (clipped 2 lines)` build/rbe/BUILD [8] `"@rules_pkg//pkg:not_compatible",` Solution Walkthrough: Before: `# .github/workflows/release.yml # build/rbe/BUILD platform( name = "rbe_platform", constraint_values = [ ..., "@rules_pkg//pkg:not_compatible", ], ... ) # .github/workflows/release.yml if ! bazel run --config=remote //release:publish_packages; then echo "Remote bazel run failed; retrying locally" bazel run --remote_executor= --remote_cache= ... //release:publish_packages fi` After: `# .github/workflows/release.yml # build/rbe/BUILD platform( name = "rbe_platform", constraint_values = [ ..., "@rules_pkg//pkg:not_compatible", ], ... ) # .github/workflows/release.yml # Bazel will automatically run incompatible actions locally due to the platform constraint. bazel run --config=remote --stamp //release:publish_packages -- "${args[@]}"` Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that the shell-level fallback is a coarse and inefficient solution, while the Bazel platform constraint is the idiomatic and more efficient way to handle action incompatibility, making the fallback redundant.	High
Possible issue	Make build fallback logic more specific Refine the `bazel` fallback logic to trigger only on specific toolchain-related errors. This prevents retrying on other failures like compilation or network issues, making the CI process more robust. .github/workflows/release.yml [240-251] -if ! bazel run --config=remote --stamp //release:publish_packages -- "${args[@]}"; then - echo "Remote bazel run failed; retrying locally without remote executor/cache" >&2 - bazel run \ - --remote_executor= \ - --remote_cache= \ - --noremote_accept_cached \ - --noremote_upload_local_results \ - --jobs=8 \ - --stamp \ - //release:publish_packages \ - -- "${args[@]}" +if ! bazel_output=$(bazel run --config=remote --stamp //release:publish_packages -- "${args[@]}" 2>&1); then + echo "$bazel_output" >&2 + # Fall back to local execution only if the failure is due to toolchain incompatibility. + # The specific error message may need to be adjusted. + if [[ "$bazel_output" == "toolchain resolution failed" ]]; then + echo "Remote bazel run failed due to toolchain issue; retrying locally without remote executor/cache" >&2 + bazel run \ + --remote_executor= \ + --remote_cache= \ + --noremote_accept_cached \ + --noremote_upload_local_results \ + --jobs=8 \ + --stamp \ + //release:publish_packages \ + -- "${args[@]}" + else + echo "Remote bazel run failed for a reason other than toolchain incompatibility." >&2 + exit 1 + fi fi Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the current fallback logic is too broad and will trigger on any `bazel` failure, potentially masking real build errors. Implementing a more specific check makes the CI pipeline more robust and reliable.	Medium
More

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1972#issuecomment-3560119709 Original created: 2025-11-20T21:29:50Z --- ## PR Code Suggestions ✨  Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion                                                                                                                                    </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>High-level</td> <td> <details><summary>Rely on Bazel's execution strategy</summary> ___ **The suggestion proposes removing the shell-level retry script from the GitHub <br>workflow. It argues that the added Bazel platform constraint is the correct and <br>more efficient method for handling toolchain incompatibilities, rendering the <br>all-or-nothing fallback redundant.** ### Examples: <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1972/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R240-R251">.github/workflows/release.yml [240-251]</a> </summary> ```yaml if ! bazel run --config=remote --stamp //release:publish_packages -- "${args[@]}"; then echo "Remote bazel run failed; retrying locally without remote executor/cache" >&2 bazel run \ --remote_executor= \ --remote_cache= \ --noremote_accept_cached \ --noremote_upload_local_results \ --jobs=8 \ --stamp \ //release:publish_packages \ ... (clipped 2 lines) ``` </details> <details> <summary> <a href="https://github.com/carverauto/serviceradar/pull/1972/files#diff-ac0d5204dc44afc495e4f6899ae668175ade69fa6b3324c94261ae2461586a21R8-R8">build/rbe/BUILD [8]</a> </summary> ```starlark "@rules_pkg//pkg:not_compatible", ``` </details> ### Solution Walkthrough: #### Before: ```starlark # .github/workflows/release.yml # build/rbe/BUILD platform( name = "rbe_platform", constraint_values = [ ..., "@rules_pkg//pkg:not_compatible", ], ... ) # .github/workflows/release.yml if ! bazel run --config=remote //release:publish_packages; then echo "Remote bazel run failed; retrying locally" bazel run --remote_executor= --remote_cache= ... //release:publish_packages fi ``` #### After: ```starlark # .github/workflows/release.yml # build/rbe/BUILD platform( name = "rbe_platform", constraint_values = [ ..., "@rules_pkg//pkg:not_compatible", ], ... ) # .github/workflows/release.yml # Bazel will automatically run incompatible actions locally due to the platform constraint. bazel run --config=remote --stamp //release:publish_packages -- "${args[@]}" ``` <details><summary>Suggestion importance[1-10]: 9</summary> __ Why: The suggestion correctly identifies that the shell-level fallback is a coarse and inefficient solution, while the Bazel platform constraint is the idiomatic and more efficient way to handle action incompatibility, making the fallback redundant. </details></details></td><td align=center>High </td></tr><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Make build fallback logic more specific</summary> ___ **Refine the <code>bazel</code> fallback logic to trigger only on specific toolchain-related <br>errors. This prevents retrying on other failures like compilation or network <br>issues, making the CI process more robust.** [.github/workflows/release.yml [240-251]](https://github.com/carverauto/serviceradar/pull/1972/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R240-R251) ```diff -if ! bazel run --config=remote --stamp //release:publish_packages -- "${args[@]}"; then - echo "Remote bazel run failed; retrying locally without remote executor/cache" >&2 - bazel run \ - --remote_executor= \ - --remote_cache= \ - --noremote_accept_cached \ - --noremote_upload_local_results \ - --jobs=8 \ - --stamp \ - //release:publish_packages \ - -- "${args[@]}" +if ! bazel_output=$(bazel run --config=remote --stamp //release:publish_packages -- "${args[@]}" 2>&1); then + echo "$bazel_output" >&2 + # Fall back to local execution only if the failure is due to toolchain incompatibility. + # The specific error message may need to be adjusted. + if [[ "$bazel_output" == *"toolchain resolution failed"* ]]; then + echo "Remote bazel run failed due to toolchain issue; retrying locally without remote executor/cache" >&2 + bazel run \ + --remote_executor= \ + --remote_cache= \ + --noremote_accept_cached \ + --noremote_upload_local_results \ + --jobs=8 \ + --stamp \ + //release:publish_packages \ + -- "${args[@]}" + else + echo "Remote bazel run failed for a reason other than toolchain incompatibility." >&2 + exit 1 + fi fi ``` - [ ] **Apply / Chat**  <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: The suggestion correctly identifies that the current fallback logic is too broad and will trigger on any `bazel` failure, potentially masking real build errors. Implementing a more specific check makes the CI pipeline more robust and reliable. </details></details></td><td align=center>Medium </td></tr> <tr><td align="center" colspan="2"> - [ ] More  </td><td></td></tr></tbody></table>