-
Notifications
You must be signed in to change notification settings - Fork 620
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
fix(build): image building #1603
Conversation
Warning Rate limit exceeded@colinlyguo has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 19 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe pull request updates the version tag in Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1603 +/- ##
========================================
Coverage 51.72% 51.72%
========================================
Files 157 157
Lines 12555 12555
========================================
Hits 6494 6494
Misses 5497 5497
Partials 564 564
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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)
build/dockerfiles/bridgehistoryapi-db-cli.Dockerfile (1)
16-16
: Changed Final Stage Base Image to Ubuntu.
The deployment stage now usesubuntu:20.04
in place of the previous alpine image. This change may offer improved compatibility with system dependencies; however, it can also lead to a larger image size. Consider reviewing whether a slimmer variant (e.g.,ubuntu:20.04-slim
) might better suit your runtime constraints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
build/dockerfiles/bridgehistoryapi-db-cli.Dockerfile
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: rollup-db-cli
- GitHub Check: rollup_relayer
- GitHub Check: gas_oracle
- GitHub Check: coordinator-cron
- GitHub Check: bridgehistoryapi-db-cli
- GitHub Check: bridgehistoryapi-api
- GitHub Check: coordinator-api
- GitHub Check: bridgehistoryapi-fetcher
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: check
- GitHub Check: check
- GitHub Check: check
- GitHub Check: tests
- GitHub Check: check
🔇 Additional comments (2)
build/dockerfiles/bridgehistoryapi-db-cli.Dockerfile (2)
2-2
: Updated Base Image for Build Dependencies.
The base image is nowscrolltech/go-rust-builder:go-1.21-rust-nightly-2023-12-03
, which integrates both Go 1.21 and Rust nightly tooling. Please verify that this image meets all build-time requirements and is fully compatible with existing dependency installations.
17-17
: Configured CGO_LDFLAGS for Linker Behavior.
The addition of the environment variableCGO_LDFLAGS="-Wl,--no-as-needed -ldl"
explicitly customizes the linker behavior for Cgo, which can be crucial for managing shared libraries. Please verify that these flags are necessary for your use-case and do not introduce unintended side effects during linking.
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)
build/dockerfiles/bridgehistoryapi-db-cli.Dockerfile (1)
16-17
: Revisit Runtime Base Image and Environment Variable Usage.
The runtime stage now usesubuntu:20.04
instead of Alpine. While Ubuntu may offer a richer library ecosystem, note the potential trade-off with a larger image size. Additionally, the environment variableCGO_LDFLAGS
is set again in this stage—ensure that this setting is intended for the runtime environment (typically, these flags are most relevant during compilation) and that the Ubuntu image includes all necessary runtime dependencies.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
build/dockerfiles/bridgehistoryapi-db-cli.Dockerfile
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (17)
- GitHub Check: rollup-db-cli
- GitHub Check: rollup_relayer
- GitHub Check: gas_oracle
- GitHub Check: coordinator-cron
- GitHub Check: bridgehistoryapi-api
- GitHub Check: coordinator-api
- GitHub Check: bridgehistoryapi-fetcher
- GitHub Check: bridgehistoryapi-db-cli
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: tests
- GitHub Check: check
- GitHub Check: tests
- GitHub Check: check
- GitHub Check: check
- GitHub Check: check
- GitHub Check: tests
🔇 Additional comments (2)
build/dockerfiles/bridgehistoryapi-db-cli.Dockerfile (2)
2-2
: Updated Base Image to Include Go and Rust Tooling.
The base image has been changed toscrolltech/go-rust-builder:go-1.21-rust-nightly-2023-12-03
, which integrates both Go and Rust toolchains. Please verify that this image meets your security, compatibility, and maintenance standards, given its role in the new build environment.
11-13
: Validate the Build Command and Linker Flags.
The build command now employs a multi-mount approach and explicitly setsCGO_LDFLAGS
to"-Wl,--no-as-needed -ldl"
when buildingdb_cli
. Double-check that these linker flags are necessary for your project requirements and that the directory/src/bridge-history-api/cmd/db_cli
is correctly structured in the build context.
52af1ef
to
e1b1490
Compare
Purpose or design rationale of this PR
This PR fixes
go.work.sync
update in image building and fixes compile errors inrollup-relayer
andgas-oracle
. It also changes the base images ofrollup-db-cli
andbridgehistoryapi-db-cli
to Ubuntu, making them compatible with.a
files inda-codec
'szstd
library.PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
Deployment tag versioning
Has
tag
incommon/version.go
been updated or have you addedbump-version
label to this PR?Breaking change label
Does this PR have the
breaking-change
label?Summary by CodeRabbit
Chores
Refactor