chore: use system rocksdb libs, if available #565
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes introduce a new section Changes
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.cargo/config.toml (1)
5-7: Environment variables added for system libraries.The addition of
ROCKSDB_LIB_DIRandSNAPPY_LIB_DIRenvironment variables aligns with the PR objective of using system-installed RocksDB libraries. This change allows the project to utilize system libraries instead of bundled versions, potentially improving flexibility and efficiency.However, hardcoding the path to "/usr/lib/" might not be suitable for all systems or environments. Consider making this more flexible by:
- Using a relative path or a placeholder that can be overridden.
- Allowing these paths to be set via external environment variables.
- Implementing a fallback mechanism if the system libraries are not found in the specified location.
Would you like assistance in implementing a more flexible approach for specifying library paths?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .cargo/config.toml (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
.cargo/config.toml (1)
4-4: Improved readability with section separation.The addition of an empty line after the
[target.thumbv7em-none-eabi]section enhances the readability of the configuration file by clearly separating different sections.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
utils/docker/Dockerfile (2)
26-27: Approve RocksDB installation with a minor suggestion.The addition of
librocksdb-devaligns well with the PR objective to use system RocksDB libraries when available. This change enhances the flexibility of the build process.Consider updating the comment to reflect that only RocksDB is being installed in this step:
-# Install rocksdb + snappy +# Install rocksdb
Line range hint
53-77: Approve runtime stage improvements with a minor suggestion.The changes in the runtime stage enhance the Docker setup by:
- Using a consistent base image (Debian-based) with the builder stage.
- Including cleanup commands to optimize the image size.
- Setting up an entrypoint script for improved container flexibility.
- Exposing port 8080 for the application.
These improvements contribute to a more efficient and maintainable Docker image.
For consistency with the builder stage, consider using
COPY --chmod=+xinstead of a separateRUN chmodcommand for the entrypoint script:-COPY utils/docker/entrypoint.sh / -RUN chmod a+rx /entrypoint.sh +COPY --chmod=+x utils/docker/entrypoint.sh /This change would make the Dockerfile more concise and consistent with modern Docker best practices.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- utils/docker/Dockerfile (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
utils/docker/Dockerfile (2)
Line range hint
39-46: Approve working directory setup and file copying.The changes in this segment are well-structured and follow best practices for Dockerfile organization. Setting up the working directory and copying necessary files are crucial steps in preparing the build environment.
Line range hint
1-77: Overall approval with positive feedback on Dockerfile improvements.The changes in this Dockerfile successfully achieve the PR objective of using system RocksDB libraries while also introducing several improvements to the build and runtime processes:
- Installation of system RocksDB libraries in the builder stage.
- Refinement of the working directory setup and file copying process.
- Optimization of the runtime stage with cleanup commands and a more flexible entrypoint setup.
These modifications contribute to a more efficient, flexible, and maintainable Docker image for the mehari application. The changes are well-structured and follow Docker best practices.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #565 +/- ##
==========================================
+ Coverage 73.99% 74.01% +0.02%
==========================================
Files 26 26
Lines 9849 9849
==========================================
+ Hits 7288 7290 +2
+ Misses 2561 2559 -2 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
New Features
ROCKSDB_LIB_DIRandSNAPPY_LIB_DIR.rocksdblibrary.Bug Fixes