-
Notifications
You must be signed in to change notification settings - Fork 8
fix: update the jar paths and thrift.bzl #317
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
Co-authored-by: Thomas Chow <[email protected]>
WalkthroughThe pull request updates the GCP artifacts build script by renaming the Bazel build commands and corresponding variable assignments to adopt a consistent naming convention with a Changes
Sequence Diagram(s)sequenceDiagram
participant F as thrift_java_library
participant OS as System Environment
F->>OS: Detect platform
alt Platform is macOS
OS-->>F: Return "/opt/homebrew/bin/thrift"
else
OS-->>F: Return "/usr/local/bin/thrift"
end
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
distribution/build_and_upload_gcp_artifacts.sh(1 hunks)tools/build_rules/thrift/thrift.bzl(1 hunks)
🔇 Additional comments (2)
distribution/build_and_upload_gcp_artifacts.sh (2)
48-49: LGTM: JAR paths updated to match new namingPath variables correctly reflect the new JAR names.
45-46: Verify bazel target names✅ Verification successful
Bazel target names are consistent.
build_and_upload_gcp_artifacts.shnow builds the correct targets.- References in
api/py/ai/chronon/repo/run.pymatch these names.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the new bazel targets exist bazel query //cloud_gcp:cloud_gcp_lib_deploy.jar bazel query //cloud_gcp:cloud_gcp_submitter_deploy.jarLength of output: 299
Script:
#!/bin/bash # Search for target definitions or references in the repository rg -n "cloud_gcp_lib_deploy\.jar" . rg -n "cloud_gcp_submitter_deploy\.jar" .Length of output: 761
| thrift_binary = select({ | ||
| "@platforms//os:macos": "/opt/homebrew/bin/thrift", | ||
| "//conditions:default": "/usr/local/bin/thrift", | ||
| }), |
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.
💡 Codebase verification
Thrift binary missing on Linux
- The script warned that
/usr/local/bin/thriftis not found on Linux. - Confirm the expected binary installation or adjust the path accordingly.
🔗 Analysis chain
Verify thrift binary paths exist
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if thrift binary exists in expected paths
if [[ "$OSTYPE" == "darwin"* ]]; then
if [ ! -f "/opt/homebrew/bin/thrift" ]; then
echo "Warning: Thrift binary not found at /opt/homebrew/bin/thrift"
fi
else
if [ ! -f "/usr/local/bin/thrift" ]; then
echo "Warning: Thrift binary not found at /usr/local/bin/thrift"
fi
fi
Length of output: 198
## Summary
## Checklist
- [ ] Added Unit Tests
- [ ] Covered by existing CI
- [ ] Integration tested
- [ ] Documentation update
<!-- av pr metadata
This information is embedded by the av CLI when creating PRs to track
the status of stacks when using Aviator. Please do not delete or edit
this section of the PR.
```
{"parent":"main","parentHead":"","trunk":"main"}
```
-->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Chores**
- Streamlined the build process with more consistent naming conventions
for cloud deployment artifacts.
- **New Features**
- Enhanced support for macOS environments by introducing
platform-specific handling during the build, ensuring improved
compatibility.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: Thomas Chow <[email protected]>
## Summary
## Checklist
- [ ] Added Unit Tests
- [ ] Covered by existing CI
- [ ] Integration tested
- [ ] Documentation update
<!-- av pr metadata
This information is embedded by the av CLI when creating PRs to track
the status of stacks when using Aviator. Please do not delete or edit
this section of the PR.
```
{"parent":"main","parentHead":"","trunk":"main"}
```
-->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Chores**
- Streamlined the build process with more consistent naming conventions
for cloud deployment artifacts.
- **New Features**
- Enhanced support for macOS environments by introducing
platform-specific handling during the build, ensuring improved
compatibility.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: Thomas Chow <[email protected]>
## Summary
## Checklist
- [ ] Added Unit Tests
- [ ] Covered by existing CI
- [ ] Integration tested
- [ ] Documentation update
<!-- av pr metadata
This information is embedded by the av CLI when creating PRs to track
the status of stacks when using Aviator. Please do not delete or edit
this section of the PR.
```
{"parent":"main","parentHead":"","trunk":"main"}
```
-->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Chores**
- Streamlined the build process with more consistent naming conventions
for cloud deployment artifacts.
- **New Features**
- Enhanced support for macOS environments by introducing
platform-specific handling during the build, ensuring improved
compatibility.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: Thomas Chow <[email protected]>
## Summary
## Checklist
- [ ] Added Unit Tests
- [ ] Covered by existing CI
- [ ] Integration tested
- [ ] Documentation update
<!-- av pr metadata
This information is embedded by the av CLI when creating PRs to track
the status of stacks when using Aviator. Please do not delete or edit
this section of the PR.
```
{"parent":"main","parentHead":"","trunk":"main"}
```
-->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Chores**
- Streamlined the build process with more consistent naming conventions
for cloud deployment artifacts.
- **New Features**
- Enhanced support for macOS environments by introducing
platform-specific handling during the build, ensuring improved
compatibility.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: Thomas Chow <[email protected]>
## Summary
## Checklist
- [ ] Added Unit Tests
- [ ] Covered by existing CI
- [ ] Integration tested
- [ ] Documentation update
<!-- av pr metadata
This information is embedded by the av CLI when creating PRs to track
the status of stacks when using Aviator. Please do not delete or edit
this section of the PR.
```
{"parent":"main","parentHead":"","trunk":"main"}
```
-->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Chores**
- Streamlined the build process with more consistent naming conventions
for cloud deployment artifacts.
- **New Features**
- Enhanced support for macOS environments by introducing
platform-specific handling during the build, ensuring improved
compatibility.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: Thomas Chow <[email protected]>
## Summary
## Cheour clientslist
- [ ] Added Unit Tests
- [ ] Covered by existing CI
- [ ] Integration tested
- [ ] Documentation update
<!-- av pr metadata
This information is embedded by the av CLI when creating PRs to traour clients
the status of staour clientss when using Aviator. Please do not delete or edit
this section of the PR.
```
{"parent":"main","parentHead":"","trunk":"main"}
```
-->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Chores**
- Streamlined the build process with more consistent naming conventions
for cloud deployment artifacts.
- **New Features**
- Enhanced support for macOS environments by introducing
platform-specific handling during the build, ensuring improved
compatibility.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: Thomas Chow <[email protected]>
Summary
Checklist
Summary by CodeRabbit