Fix: Disable Bazel Remote Cache #919
Conversation
WalkthroughBazel remote cache and Google credentials flags were removed from all GitHub Actions workflow Bazel build and test commands. Credential setup steps were deleted. Bazel commands now run without remote caching or authentication across all workflows. No build targets or public entities were changed. Changes
Possibly related PRs
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
.github/workflows/test_bazel_config.yaml (1)
43-46: Secrets step will break for fork PRsGitHub won’t pass
${{ secrets.BAZEL_CACHE_CREDENTIALS }}to forked PRs, sobase64 -dmay error out before reaching your new guard.
Add a workflow-level condition to skip this step for forked PRs:- name: Setup Bazel cache credentials +if: ${{ !github.event.pull_request.head.repo.fork }} run: | echo "${{ secrets.BAZEL_CACHE_CREDENTIALS }}" | base64 -d > bazel-cache-key.json.github/workflows/push_to_canary.yaml (1)
33-36: Guard credential creationSkip this step for fork PRs to avoid secret leakage / build failure.
♻️ Duplicate comments (13)
.github/workflows/test_scala_fmt.yaml (1)
41-44: Skip credentials step for forksReplicate the conditional guard so forks don’t hit a secret they can’t read (see prior comment for exact snippet).
.github/workflows/test_scala_2_12_non_spark.yaml (7)
84-92: Same duplication comment applies.
113-121: Same duplication comment applies.
142-150: Same duplication comment applies.
171-180: Same duplication comment applies.
201-210: Same duplication comment applies.
231-240: Same duplication comment applies.
261-272: Same duplication comment applies..github/workflows/test_scala_2_12_spark.yaml (5)
67-78: Duplication note applies.
100-111: Duplication note applies.
133-144: Duplication note applies.
166-177: Duplication note applies.
199-210: Duplication note applies.
🧹 Nitpick comments (4)
.github/workflows/test_scala_fmt.yaml (1)
47-55: Same condition pitfalls & missing final newline
- Prefer the simpler fork flag as noted earlier.
- YAML-lint flags the missing newline at EOF; add one to avoid noisy CI.
.github/workflows/test_scala_2_12_non_spark.yaml (1)
55-63: Factor out duplicated fork checkThe same
ifblock is repeated in every test job. Extract to a reusable Bash function or a composite action to cut ~70 duplicated lines and future-proof changes.Example:
run_bazel_test() { local target=$1 if [ "${{ github.event.pull_request.head.repo.fork }}" = "true" ]; then bazel test "$target" else bazel test --remote_cache=... --google_credentials=bazel-cache-key.json "$target" fi }.github/workflows/test_scala_2_12_spark.yaml (2)
36-46: Inline fork flag & avoid duplicate logicUse the shorter flag check (
github.event.pull_request.head.repo.fork) and consider extracting a helper as noted in non-spark workflow to avoid copy-paste across seven jobs.
232-243: Add final newline to satisfy YAML-lint.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/push_to_canary.yaml(1 hunks).github/workflows/test_bazel_config.yaml(1 hunks).github/workflows/test_scala_2_12_non_spark.yaml(8 hunks).github/workflows/test_scala_2_12_spark.yaml(7 hunks).github/workflows/test_scala_fmt.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: ken-zlai
PR: zipline-ai/chronon#185
File: .github/workflows/test_frontend.yaml:41-42
Timestamp: 2025-01-08T18:29:51.176Z
Learning: In GitHub Actions, the `version` parameter in `awalsh128/cache-apt-pkgs-action` is a cache control number used to invalidate and regenerate the cache. It does not specify the package version - the actual package version is determined by the Ubuntu package repositories.
.github/workflows/test_bazel_config.yaml (1)
Learnt from: ken-zlai
PR: zipline-ai/chronon#185
File: .github/workflows/test_frontend.yaml:41-42
Timestamp: 2025-01-08T18:29:51.176Z
Learning: In GitHub Actions, the `version` parameter in `awalsh128/cache-apt-pkgs-action` is a cache control number used to invalidate and regenerate the cache. It does not specify the package version - the actual package version is determined by the Ubuntu package repositories.
.github/workflows/push_to_canary.yaml (2)
Learnt from: ken-zlai
PR: zipline-ai/chronon#185
File: .github/workflows/test_frontend.yaml:41-42
Timestamp: 2025-01-08T18:29:51.176Z
Learning: In GitHub Actions, the `version` parameter in `awalsh128/cache-apt-pkgs-action` is a cache control number used to invalidate and regenerate the cache. It does not specify the package version - the actual package version is determined by the Ubuntu package repositories.
Learnt from: tchow-zlai
PR: zipline-ai/chronon#393
File: cloud_gcp/BUILD.bazel:99-99
Timestamp: 2025-02-22T20:30:28.381Z
Learning: The jar file "iceberg-bigquery-catalog-1.5.2-1.0.1-beta.jar" in cloud_gcp/BUILD.bazel is a local dependency and should not be replaced with maven_artifact.
.github/workflows/test_scala_2_12_spark.yaml (1)
Learnt from: ken-zlai
PR: zipline-ai/chronon#185
File: .github/workflows/test_frontend.yaml:41-42
Timestamp: 2025-01-08T18:29:51.176Z
Learning: In GitHub Actions, the `version` parameter in `awalsh128/cache-apt-pkgs-action` is a cache control number used to invalidate and regenerate the cache. It does not specify the package version - the actual package version is determined by the Ubuntu package repositories.
🪛 YAMLlint (1.37.1)
.github/workflows/test_scala_fmt.yaml
[error] 55-55: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/push_to_canary.yaml
[error] 101-101: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
[error] 112-112: trailing spaces
(trailing-spaces)
[error] 115-115: trailing spaces
(trailing-spaces)
.github/workflows/test_scala_2_12_spark.yaml
[error] 243-243: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: cloud_aws_tests
- GitHub Check: online_tests
- GitHub Check: cloud_gcp_tests
- GitHub Check: service_tests
- GitHub Check: api_tests
- GitHub Check: aggregator_tests
- GitHub Check: join_tests
- GitHub Check: flink_tests
- GitHub Check: streaming_tests
- GitHub Check: service_commons_tests
- GitHub Check: analyzer_tests
- GitHub Check: groupby_tests
- GitHub Check: spark_tests
- GitHub Check: batch_tests
- GitHub Check: scala_compile_fmt_fix
| if [ "${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork }}" = "true" ]; then | ||
| bazel build \ | ||
| --nobuild \ | ||
| //... | ||
| else | ||
| bazel build \ | ||
| --remote_cache=https://storage.googleapis.com/zipline-bazel-cache \ | ||
| --google_credentials=bazel-cache-key.json \ | ||
| --nobuild \ | ||
| //... | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Condition syntax brittle; use direct fork flag
github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork
works, but expands to the literal true/false string at YAML-render time.
You can simplify and avoid the extra test by keying directly off the fork flag:
-if [ "${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork }}" = "true" ]; then
+if [ "${{ github.event.pull_request.head.repo.fork }}" = "true" ]; thenThis removes a redundant comparison and avoids surprises if GitHub ever changes expression precedence.
🤖 Prompt for AI Agents
In .github/workflows/test_bazel_config.yaml around lines 49 to 59, the condition
checking if the event is a pull request from a fork uses a brittle syntax that
expands to literal strings and includes a redundant comparison. Simplify the
condition by directly using the fork flag without comparing it to "true".
Replace the entire condition with a direct check of the fork flag to make the
logic clearer and more robust.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/push_to_canary.yaml (1)
33-36: Secrets step still runs for forked PRs – will fail before your new logic can help
When the workflow runs on a fork,${{ secrets.BAZEL_CACHE_CREDENTIALS }}is empty.
base64 -dthen exits 1 and the job never reaches the conditional Bazel block you added.Add an
if:guard (or move the whole step into the remote-cache branch):- - name: Setup Bazel cache credentials - run: | - echo "${{ secrets.BAZEL_CACHE_CREDENTIALS }}" | base64 -d > bazel-cache-key.json + - name: Setup Bazel cache credentials + if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork != true }} + run: | + echo "${{ secrets.BAZEL_CACHE_CREDENTIALS }}" | base64 -d > bazel-cache-key.json
🧹 Nitpick comments (3)
.github/workflows/push_to_canary.yaml (3)
100-107: Guard expression is fine, but string-compare is redundant
${{ … }}already yields'true'/'false'.
The extra[...] = "true"can be dropped:-if [ "${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork }}" = "true" ]; then +if ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork }}; thenCleaner and one less subshell.
101-126: Five nearly identical Bazel invocations – consider a loopReduces copy-paste, avoids future drift:
-bazel build //flink:flink_assembly_deploy.jar -... -bazel build //flink:connectors_pubsub_deploy.jar +targets=( + //flink:flink_assembly_deploy.jar + //cloud_gcp:cloud_gcp_lib_deploy.jar + //cloud_aws:cloud_aws_lib_deploy.jar + //service:service_assembly_deploy.jar + //flink:connectors_pubsub_deploy.jar +) +for t in "${targets[@]}"; do + bazel build $t +doneSame idea for the remote-cache branch (prepend the two flags once).
101-125: Trailing spaces flagged by YAMLlint
Lines 101, 109, 112, 115, 118, 121, 124 have stray spaces → failyaml-lint.
Quick fix: delete them.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/push_to_canary.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ken-zlai
PR: zipline-ai/chronon#185
File: .github/workflows/test_frontend.yaml:41-42
Timestamp: 2025-01-08T18:29:51.176Z
Learning: In GitHub Actions, the `version` parameter in `awalsh128/cache-apt-pkgs-action` is a cache control number used to invalidate and regenerate the cache. It does not specify the package version - the actual package version is determined by the Ubuntu package repositories.
.github/workflows/push_to_canary.yaml (2)
Learnt from: ken-zlai
PR: zipline-ai/chronon#185
File: .github/workflows/test_frontend.yaml:41-42
Timestamp: 2025-01-08T18:29:51.176Z
Learning: In GitHub Actions, the `version` parameter in `awalsh128/cache-apt-pkgs-action` is a cache control number used to invalidate and regenerate the cache. It does not specify the package version - the actual package version is determined by the Ubuntu package repositories.
Learnt from: tchow-zlai
PR: zipline-ai/chronon#393
File: cloud_gcp/BUILD.bazel:99-99
Timestamp: 2025-02-22T20:30:28.381Z
Learning: The jar file "iceberg-bigquery-catalog-1.5.2-1.0.1-beta.jar" in cloud_gcp/BUILD.bazel is a local dependency and should not be replaced with maven_artifact.
🪛 YAMLlint (1.37.1)
.github/workflows/push_to_canary.yaml
[error] 101-101: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: streaming_tests
- GitHub Check: batch_tests
- GitHub Check: analyzer_tests
- GitHub Check: fetcher_tests
- GitHub Check: spark_tests
- GitHub Check: join_tests
- GitHub Check: cloud_aws_tests
- GitHub Check: cloud_gcp_tests
- GitHub Check: scala_compile_fmt_fix
- GitHub Check: service_tests
- GitHub Check: api_tests
- GitHub Check: service_commons_tests
- GitHub Check: aggregator_tests
- GitHub Check: online_tests
- GitHub Check: flink_tests
- GitHub Check: enforce_triggered_workflows
piyush-zlai
left a comment
piyush-zlai
left a comment
There was a problem hiding this comment.
Could just turn off the bazel cache - not sure if its giving us a lot perf wise
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
.github/workflows/test_bazel_config.yaml (1)
43-46: Dead code: cache-credential step now pointless (and breaks forks)
Remote-cache flags were removed, yet this step still unconditionally decodes the secret. It:
- Leaks an unnecessary secret to runners.
- Fails on forked PRs (no secret → empty file).
Drop the step or guard it behind a non-fork condition.
- - name: Setup Bazel cache credentials - run: | - echo "${{ secrets.BAZEL_CACHE_CREDENTIALS }}" | base64 -d > bazel-cache-key.json
🧹 Nitpick comments (2)
.github/workflows/test_bazel_config.yaml (1)
47-51: Misleading step name + missing newline
Step says “Run bazel sync” but actually runsbazel build --nobuild. Rename for clarity.
Also add a trailing newline to silence YAML-lint.- - name: Run bazel sync + - name: Verify Bazel config (nobuild) … +# (newline).github/workflows/test_scala_2_12_non_spark.yaml (1)
182-182: Add EOF newline
Missing final newline triggers YAML-lintnew-line-at-end-of-file.- //cloud_aws:tests +\ //cloud_aws:tests +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/push_to_canary.yaml(1 hunks).github/workflows/test_bazel_config.yaml(1 hunks).github/workflows/test_scala_2_12_non_spark.yaml(3 hunks).github/workflows/test_scala_2_12_spark.yaml(4 hunks).github/workflows/test_scala_fmt.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/test_scala_fmt.yaml
- .github/workflows/test_scala_2_12_spark.yaml
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ken-zlai
PR: zipline-ai/chronon#185
File: .github/workflows/test_frontend.yaml:41-42
Timestamp: 2025-01-08T18:29:51.176Z
Learning: In GitHub Actions, the `version` parameter in `awalsh128/cache-apt-pkgs-action` is a cache control number used to invalidate and regenerate the cache. It does not specify the package version - the actual package version is determined by the Ubuntu package repositories.
.github/workflows/push_to_canary.yaml (2)
Learnt from: chewy-zlai
PR: zipline-ai/chronon#47
File: docker-init/Dockerfile:36-38
Timestamp: 2024-10-17T01:09:24.653Z
Learning: The JAR files `spark-assembly-0.1.0-SNAPSHOT.jar` and `cloud_aws-assembly-0.1.0-SNAPSHOT.jar` are generated by `sbt` and located in the `target` directory after the build.
Learnt from: tchow-zlai
PR: zipline-ai/chronon#393
File: cloud_gcp/BUILD.bazel:99-99
Timestamp: 2025-02-22T20:30:28.381Z
Learning: The jar file "iceberg-bigquery-catalog-1.5.2-1.0.1-beta.jar" in cloud_gcp/BUILD.bazel is a local dependency and should not be replaced with maven_artifact.
.github/workflows/test_scala_2_12_non_spark.yaml (1)
Learnt from: tchow-zlai
PR: zipline-ai/chronon#393
File: cloud_gcp/BUILD.bazel:99-99
Timestamp: 2025-02-22T20:30:28.381Z
Learning: The jar file "iceberg-bigquery-catalog-1.5.2-1.0.1-beta.jar" in cloud_gcp/BUILD.bazel is a local dependency and should not be replaced with maven_artifact.
🪛 YAMLlint (1.37.1)
.github/workflows/push_to_canary.yaml
[error] 100-100: trailing spaces
(trailing-spaces)
.github/workflows/test_bazel_config.yaml
[error] 51-51: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/test_scala_2_12_non_spark.yaml
[error] 182-182: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: cloud_aws_tests
- GitHub Check: cloud_gcp_tests
- GitHub Check: service_tests
- GitHub Check: aggregator_tests
- GitHub Check: analyzer_tests
- GitHub Check: join_tests
- GitHub Check: service_commons_tests
- GitHub Check: spark_tests
- GitHub Check: groupby_tests
- GitHub Check: streaming_tests
- GitHub Check: api_tests
- GitHub Check: online_tests
- GitHub Check: flink_tests
- GitHub Check: fetcher_tests
- GitHub Check: batch_tests
- GitHub Check: scala_compile_fmt_fix
- GitHub Check: enforce_triggered_workflows
🔇 Additional comments (2)
.github/workflows/push_to_canary.yaml (2)
33-36: Redundant secret decode
Remote cache flags are gone, so the decodedbazel-cache-key.jsonis unused. Kill the whole block to speed up jobs and avoid secret exposure.[ suggest_optional_refactor ]
96-104: Trim trailing spaces
Lines 100-104 have trailing blanks; YAML-lint is shouting. Quick cleanup keeps CI green.[ suggest_nitpick ]
chewy-zlai
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/test_scala_2_12_non_spark.yaml (2)
35-178: Consider DRY-ing the 8 almost-identical jobsThe workflow repeats the same container + checkout +
bazel testpattern. A matrix strategy (or a reusable composite action) would cut ~150 lines and ease future updates.
181-181: Missing newline at EOFAdd a trailing newline to satisfy YAMLlint.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/test_scala_2_12_non_spark.yaml(1 hunks).github/workflows/test_scala_2_12_spark.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/test_scala_2_12_spark.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ken-zlai
PR: zipline-ai/chronon#185
File: .github/workflows/test_frontend.yaml:41-42
Timestamp: 2025-01-08T18:29:51.176Z
Learning: In GitHub Actions, the `version` parameter in `awalsh128/cache-apt-pkgs-action` is a cache control number used to invalidate and regenerate the cache. It does not specify the package version - the actual package version is determined by the Ubuntu package repositories.
.github/workflows/test_scala_2_12_non_spark.yaml (1)
Learnt from: tchow-zlai
PR: zipline-ai/chronon#393
File: cloud_gcp/BUILD.bazel:99-99
Timestamp: 2025-02-22T20:30:28.381Z
Learning: The jar file "iceberg-bigquery-catalog-1.5.2-1.0.1-beta.jar" in cloud_gcp/BUILD.bazel is a local dependency and should not be replaced with maven_artifact.
🪛 YAMLlint (1.37.1)
.github/workflows/test_scala_2_12_non_spark.yaml
[error] 181-181: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: analyzer_tests
- GitHub Check: streaming_tests
- GitHub Check: groupby_tests
- GitHub Check: spark_tests
- GitHub Check: batch_tests
- GitHub Check: fetcher_tests
- GitHub Check: join_tests
- GitHub Check: api_tests
- GitHub Check: cloud_aws_tests
- GitHub Check: cloud_gcp_tests
- GitHub Check: flink_tests
- GitHub Check: service_commons_tests
- GitHub Check: aggregator_tests
- GitHub Check: service_tests
- GitHub Check: online_tests
- GitHub Check: scala_compile_fmt_fix
- GitHub Check: enforce_triggered_workflows
🔇 Additional comments (2)
.github/workflows/test_scala_2_12_non_spark.yaml (2)
49-53: Remote-cache flags successfully strippedAll Bazel invocations now run without
--remote_cache/--google_credentials, resolving the secret-exposure issue.Also applies to: 68-72, 86-89, 104-107, 122-125, 140-143, 158-161, 176-181
178-181: Java 17 flags only oncloud_aws_tests– intentional?If all modules build with Java 17, add the flags to every job; if only AWS needs them, add a clarifying comment.
Summary
No longer use bazel remote cache as it requires using a secret which isn't available to external collaborators.
Checklist
Summary by CodeRabbit
Summary by CodeRabbit