Skip to content

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

Merged
merged 4 commits into from
Feb 7, 2025

Conversation

kumar-zlai
Copy link
Contributor

@kumar-zlai kumar-zlai commented Feb 6, 2025

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

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.

Copy link

coderabbitai bot commented Feb 6, 2025

Walkthrough

This 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

File(s) Change Summary
.github/workflows/test_bazel_config.yaml, .github/workflows/test_scala_non_spark.yaml Added new Bazel config workflow with concurrency settings and updated non-spark workflow to include additional module paths (hub, orchestration, service, cloud_gcp) with extra Bazel test steps.
cloud_gcp/.../BigQueryCatalogTest.scala Modified the dynamic class loading test to be ignored, thus preventing its execution in the test suite.

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
Loading
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
Loading

Possibly related PRs

Suggested reviewers

  • nikhil-zlai
  • piyush-zlai
  • varant-zlai

Poem

In code’s quiet hum, a workflow awakes,
New tests align as each commit takes,
Bazel paths converge in a digital art,
Ignored tests rest, playing their part,
A merry dance of code—a fresh start!
🚀💻🎉

Warning

Review ran into problems

🔥 Problems

GitHub 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.


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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)

📥 Commits

Reviewing files that changed from the base of the PR and between 242a62f and 4cb82e8.

📒 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.

Copy link

@coderabbitai coderabbitai bot left a 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)

📥 Commits

Reviewing files that changed from the base of the PR and between b866e93 and 0f99ee1.

📒 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

@kumar-zlai kumar-zlai merged commit f501bc6 into main Feb 7, 2025
7 checks passed
@kumar-zlai kumar-zlai deleted the bazel_sync_as_ci_check branch February 7, 2025 00:06
@coderabbitai coderabbitai bot mentioned this pull request Feb 21, 2025
4 tasks
kumar-zlai added a commit that referenced this pull request Apr 25, 2025
## 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 -->
kumar-zlai added a commit that referenced this pull request Apr 29, 2025
## 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 -->
chewy-zlai pushed a commit that referenced this pull request May 15, 2025
## 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 -->
chewy-zlai pushed a commit that referenced this pull request May 15, 2025
## 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 -->
chewy-zlai pushed a commit that referenced this pull request May 16, 2025
## 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 -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants