-
Notifications
You must be signed in to change notification settings - Fork 79
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 docker build error (benchmark-job, loadtest) #2819
Fix docker build error (benchmark-job, loadtest) #2819
Conversation
📝 WalkthroughWalkthroughThis pull request involves modifications to multiple Dockerfiles and a Makefile related to HDF5 library dependencies. The changes consistently remove the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
[CHATOPS:HELP] ChatOps commands.
|
78c5840
to
032e742
Compare
Deploying vald with
|
Latest commit: |
19d4ccc
|
Status: | ✅ Deploy successful! |
Preview URL: | https://a1d1bd82.vald.pages.dev |
Branch Preview URL: | https://bugfix-docker-fix-benchmark.vald.pages.dev |
16cb4ad
to
41e1092
Compare
Signed-off-by: vankichi <[email protected]>
41e1092
to
a728c95
Compare
Signed-off-by: vankichi <[email protected]>
@coderabbitai please review |
@vankichi I've reviewed the changes. The modifications look good and properly address the Docker build errors by:
The changes are well-structured and should resolve the build issues for benchmark-job and loadtest components. 🔗 Analysis chain🏁 Scripts executedThe following scripts were executed for the analysis: Script:
Length of output: 1621 Script:
Length of output: 72451 ✅ Actions performedComments resolved and changes approved. |
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)
Makefile.d/tools.mk (1)
262-263
: LGTM! The addition ofldconfig
is essential for proper library registration.The addition of
ldconfig
after HDF5 installation is crucial as it updates the dynamic linker cache, ensuring that the newly installed HDF5 libraries are properly registered in the system. This change complements the removal oflibhdf5-dev
package from Dockerfiles and follows best practices for installing libraries from source.This change helps prevent potential library loading issues during runtime by ensuring that the dynamic linker can locate the HDF5 libraries correctly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
hack/docker/gen/main.go
is excluded by!**/gen/**
📒 Files selected for processing (6)
Makefile.d/tools.mk
(1 hunks)dockers/ci/base/Dockerfile
(0 hunks)dockers/dev/Dockerfile
(0 hunks)dockers/example/client/Dockerfile
(0 hunks)dockers/tools/benchmark/job/Dockerfile
(0 hunks)dockers/tools/cli/loadtest/Dockerfile
(0 hunks)
💤 Files with no reviewable changes (5)
- dockers/tools/benchmark/job/Dockerfile
- dockers/example/client/Dockerfile
- dockers/dev/Dockerfile
- dockers/ci/base/Dockerfile
- dockers/tools/cli/loadtest/Dockerfile
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.
LGTM
* 💚 Add libcurl4-openssl-dev Signed-off-by: vankichi <[email protected]> * ♻️ Remove apt get libhd5-dev Signed-off-by: vankichi <[email protected]> --------- Signed-off-by: vankichi <[email protected]>
* 💚 Add libcurl4-openssl-dev * ♻️ Remove apt get libhd5-dev --------- Signed-off-by: vankichi <[email protected]> Co-authored-by: Kiichiro YUKAWA <[email protected]>
* 💚 Add libcurl4-openssl-dev Signed-off-by: vankichi <[email protected]> * ♻️ Remove apt get libhd5-dev Signed-off-by: vankichi <[email protected]> --------- Signed-off-by: vankichi <[email protected]> Signed-off-by: kpango <[email protected]>
[BUGFIX] add Health Check for Range over gRPC Connection Loop Signed-off-by: kpango <[email protected]>
Description
SSIA
Related Issue
Versions
Checklist
Special notes for your reviewer
Summary by CodeRabbit
libhdf5-dev
package from multiple Dockerfiles across different project environmentsldconfig
after HDF5 installation to refresh dynamic linker cache