-
Notifications
You must be signed in to change notification settings - Fork 0
Add new github workflow for bazel sync check #342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update adds a new GitHub Actions workflow for testing Bazel configurations and expands the existing Scala non-spark test workflow with new module paths and test steps. Additionally, a test for dynamic GCP provider loading has been modified to be ignored. The changes adjust the CI/CD pipelines and test coverage without modifying public API declarations. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GHA as GitHub Actions
participant Cont as Container
Dev->>GHA: Push/PR with Bazel config changes
GHA->>Cont: Trigger bazel_config_tests job
Cont->>Cont: Checkout repo, set up credentials
Cont->>Cont: Execute Bazel sync
Cont->>GHA: Return job result
sequenceDiagram
participant Dev as Developer
participant GHA as GitHub Actions
Dev->>GHA: Push/PR with changes in hub, orchestration, service, cloud_gcp
GHA->>GHA: Trigger non_spark_tests job
GHA->>GHA: Run Bazel tests for hub module
GHA->>GHA: Run Bazel tests for service module
GHA->>GHA: Run Bazel tests for orchestration module
GHA->>GHA: Run Bazel tests for cloud_gcp module
GHA->>Dev: Report test results
Possibly related PRs
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsGitHub Actions and Pipeline Checks: Resource not accessible by integration - https://docs.github.com/rest/actions/workflow-runs#list-workflow-runs-for-a-repository. Please grant the required permissions to the CodeRabbit GitHub App under the organization or repository settings. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test_scala_non_spark.yaml (1)
99-105
: Naming inconsistency detected.
The step is titled “Run orchestrator tests” while the target is “//orchestration:tests”. Consider renaming for consistency.Proposed change:
- - name: Run orchestrator tests + - name: Run orchestration tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
.github/workflows/test_bazel_config.yaml
(1 hunks).github/workflows/test_scala_non_spark.yaml
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: non_spark_tests
- GitHub Check: non_spark_tests
- GitHub Check: enforce_triggered_workflows
🔇 Additional comments (15)
.github/workflows/test_bazel_config.yaml (6)
1-2
: Workflow name is clear.
All good with the descriptive workflow name.
3-22
: Trigger configuration looks solid.
Push and PR triggers cover key Bazel config files and tools.
24-27
: Concurrency settings OK.
Cancel-in-progress option is appropriately used.
28-39
: Job container setup is proper.
Container image and credentials are configured; verify the image’s availability.
40-46
: Credential setup is concise.
Decoding the Bazel cache credential is correctly handled.
47-54
: Bazel sync command is intentional.
Using “bazel build --nobuild” correctly mimics a sync; double-check remote cache details..github/workflows/test_scala_non_spark.yaml (9)
1-2
: Workflow name is clear.
Name accurately reflects non-spark Scala module tests.
3-17
: Triggers now include added module paths.
New paths for hub, orchestration, service, and cloud_gcp are correctly included.Also applies to: 18-31
34-37
: Concurrency settings look good.
Cancellation behavior is properly configured.
38-49
: Job and container configuration is solid.
Settings align with the overall CI flow; confirm container image updates if needed.
50-56
: Setup steps are clean.
Checkout and credential decoding steps are concise and effective.
57-84
: Standard test steps verified.
Flink, Aggregator, Online, and API test commands use consistent Bazel flags.
85-91
: Hub tests step is clear.
Bazel command for hub tests is consistent with setup.
92-98
: Service tests step is fine.
Bazel command matches repository structure.
106-112
: Cloud GCP tests added correctly.
Bazel test command is applied appropriately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cloud_gcp/src/test/scala/ai/chronon/integrations/cloud_gcp/BigQueryCatalogTest.scala (1)
48-53
: Consider adding a TODO comment.Add a comment explaining why the test is ignored and when it should be re-enabled.
- it should "verify dynamic classloading of GCP providers" ignore { + // TODO: Temporarily ignored because <reason>. Re-enable when <condition> + it should "verify dynamic classloading of GCP providers" ignore {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
cloud_gcp/src/test/scala/ai/chronon/integrations/cloud_gcp/BigQueryCatalogTest.scala
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: bazel_config_tests
- GitHub Check: non_spark_tests
- GitHub Check: scala_compile_fmt_fix
- GitHub Check: enforce_triggered_workflows
## Summary Added a new workflow to verify the bazel config setup. This essentially validates all our bazel config by pulling all the necessary dependencies for all targets with out actually building them which is similar to the `Sync project` option in IntelliJ. This should help us capture errors in our bazel config setup for the CI. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Chores - Added a new automated CI workflow to run configuration tests with improved concurrency management. - Expanded CI triggers to include additional modules for broader testing coverage. - Tests - Temporarily disabled a dynamic class loading test in the cloud integrations to improve overall test stability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Added a new workflow to verify the bazel config setup. This essentially validates all our bazel config by pulling all the necessary dependencies for all targets with out actually building them which is similar to the `Sync project` option in IntelliJ. This should help us capture errors in our bazel config setup for the CI. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Chores - Added a new automated CI workflow to run configuration tests with improved concurrency management. - Expanded CI triggers to include additional modules for broader testing coverage. - Tests - Temporarily disabled a dynamic class loading test in the cloud integrations to improve overall test stability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Added a new workflow to verify the bazel config setup. This essentially validates all our bazel config by pulling all the necessary dependencies for all targets with out actually building them which is similar to the `Sync project` option in IntelliJ. This should help us capture errors in our bazel config setup for the CI. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Chores - Added a new automated CI workflow to run configuration tests with improved concurrency management. - Expanded CI triggers to include additional modules for broader testing coverage. - Tests - Temporarily disabled a dynamic class loading test in the cloud integrations to improve overall test stability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Added a new workflow to verify the bazel config setup. This essentially validates all our bazel config by pulling all the necessary dependencies for all targets with out actually building them which is similar to the `Sync project` option in IntelliJ. This should help us capture errors in our bazel config setup for the CI. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Chores - Added a new automated CI workflow to run configuration tests with improved concurrency management. - Expanded CI triggers to include additional modules for broader testing coverage. - Tests - Temporarily disabled a dynamic class loading test in the cloud integrations to improve overall test stability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Added a new workflow to verify the bazel config setup. This essentially validates all our bazel config by pulling all the necessary dependencies for all targets with out actually building them which is similar to the `Sync project` option in IntelliJ. This should help us capture errors in our bazel config setup for the CI. ## Cheour clientslist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Chores - Added a new automated CI workflow to run configuration tests with improved concurrency management. - Expanded CI triggers to include additional modules for broader testing coverage. - Tests - Temporarily disabled a dynamic class loading test in the cloud integrations to improve overall test stability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Added a new workflow to verify the bazel config setup. This essentially validates all our bazel config by pulling all the necessary dependencies for all targets with out actually building them which is similar to the
Sync project
option in IntelliJ. This should help us capture errors in our bazel config setup for the CI.Checklist
Summary by CodeRabbit