build: add dev container for local development #5292
Conversation
Add IDE compatible dev container setup for easier local development and testing. Files added: - .devcontainer/Dockerfile: Build environment based on upstream docker-images/build-ubuntu24, with mariadb-client added for testing - .devcontainer/docker-compose.yaml: Dev container + MariaDB service - .devcontainer/devcontainer.json: IDE configuration Usage with IDE: https://containers.dev/ Usage with docker compose: docker compose -f .devcontainer/docker-compose.yaml up -d docker compose -f .devcontainer/docker-compose.yaml exec dev make The MariaDB service is available at hostname "mariadb" for testing.
Summary of ChangesHello @rophy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request aims to significantly improve the developer onboarding and local development experience by providing a ready-to-use dev container environment. It encapsulates all necessary build tools and dependencies, along with a testing database, into a consistent and isolated setup, thereby reducing environment setup friction and ensuring a uniform development baseline for all contributors. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughThese changes introduce development container infrastructure for ProxySQL, including a Dockerfile for the build environment, a DevContainer configuration for VSCode integration, Docker Compose services for dev and MariaDB, and updated documentation detailing the recommended Dev Container workflow and manual setup instructions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a dev container setup, which is a great addition for simplifying local development. The configuration is well-structured. My review includes a few suggestions to improve the Dockerfile for better efficiency and reproducibility, and a recommendation to avoid hardcoded credentials in the docker-compose file for better security practices.
| RUN apt update && \ | ||
| apt full-upgrade -y | ||
|
|
||
| # gcc & build tools | ||
| RUN apt install -y \ | ||
| make cmake automake bison flex \ | ||
| git wget \ | ||
| gcc g++ \ | ||
| libtool \ | ||
| gdb gdbserver \ | ||
| equivs \ | ||
| python3 \ | ||
| zstd \ | ||
| pkg-config \ | ||
| sudo | ||
|
|
||
| # proxysql build dependencies | ||
| RUN apt install -y \ | ||
| libssl-dev gnutls-dev libgnutls28-dev \ | ||
| libmysqlclient-dev \ | ||
| libunwind8 libunwind-dev \ | ||
| uuid-dev \ | ||
| libncurses-dev \ | ||
| libicu-dev \ | ||
| libevent-dev \ | ||
| libtirpc-dev | ||
|
|
||
| # debug images need valgrind | ||
| RUN apt install -y \ | ||
| valgrind | ||
|
|
||
| # mysql client for testing | ||
| RUN apt install -y \ | ||
| mariadb-client | ||
|
|
||
| # clean apt cache | ||
| RUN apt clean && \ | ||
| rm -rf /var/cache/apt/* && \ | ||
| rm -rf /var/lib/apt/lists/* |
There was a problem hiding this comment.
To optimize the Docker image size and build speed, it's a best practice to chain all apt operations into a single RUN command. This avoids creating multiple intermediate layers. Combining apt-get update, apt-get install, and the cleanup steps (apt-get clean, rm) in one layer ensures that the package cache is cleaned up in the same layer it was created, effectively reducing the final image size. I've also added --no-install-recommends which is a common practice to further reduce size.
RUN apt-get update && \
apt-get full-upgrade -y && \
apt-get install -y --no-install-recommends \
# gcc & build tools
make cmake automake bison flex \
git wget \
gcc g++ \
libtool \
gdb gdbserver \
equivs \
python3 \
zstd \
pkg-config \
sudo \
# proxysql build dependencies
libssl-dev gnutls-dev libgnutls28-dev \
libmysqlclient-dev \
libunwind8 libunwind-dev \
uuid-dev \
libncurses-dev \
libicu-dev \
libevent-dev \
libtirpc-dev \
# debug images need valgrind
valgrind \
# mysql client for testing
mariadb-client && \
# clean apt cache
apt-get clean && \
rm -rf /var/lib/apt/lists/*
There was a problem hiding this comment.
LGTM. Though the current version honors original Dockerfile mentioned in CONTRIBUTING.md.
I'll let proxysql team comment if we want to optimize the Dockerfile.
| rm -rf /var/lib/apt/lists/* | ||
|
|
||
| # fetch needed include | ||
| RUN wget -q -O /usr/include/mysql/hash.h https://raw.githubusercontent.com/mysql/mysql-server/5.7/include/hash.h |
There was a problem hiding this comment.
For better reproducibility and to avoid potential issues if the 5.7 tag is ever moved or deleted, it's recommended to pin the downloaded file to a specific commit hash instead of a tag. The tag 5.7 currently points to commit b339a7d72f845a1343355650454457845e63ea37.
RUN wget -q -O /usr/include/mysql/hash.h https://raw.githubusercontent.com/mysql/mysql-server/b339a7d72f845a1343355650454457845e63ea37/include/hash.h
| mariadb: | ||
| image: mariadb:11.4 | ||
| environment: | ||
| MYSQL_ROOT_PASSWORD: root |
There was a problem hiding this comment.
The MYSQL_ROOT_PASSWORD is hardcoded in the docker-compose.yaml file. While this is for a local development environment, it's a security risk to commit credentials directly into source control. A better practice is to use environment variable substitution. You can then define the password in a .env file (which should be added to .gitignore) at the project root.
Example .env file:
MYSQL_ROOT_PASSWORD=root
Docker Compose will automatically pick up this file.
MYSQL_ROOT_PASSWORD: ${MYSQL_ROOT_PASSWORD}There was a problem hiding this comment.
This follows the existing project convention - all docker-compose files under docker/ use hardcoded MYSQL_ROOT_PASSWORD: root for local dev/test environments.
![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
🤖 Fix all issues with AI agents
In @CONTRIBUTING.md:
- Around line 43-45: Replace the bare URL on the first sentence with a markdown
link to match the style used on the next line: change the plain
https://github.com/ProxySQL/docker-images/tree/main/build-images to a markdown
link like [ProxySQL docker-images
build-images](https://github.com/ProxySQL/docker-images/tree/main/build-images)
in CONTRIBUTING.md so both references use consistent markdown link syntax and
satisfy markdownlint MD034.
🧹 Nitpick comments (2)
.devcontainer/docker-compose.yaml (1)
12-18: Consider adding a host port offset to avoid conflicts.Exposing
3306:3306may conflict with a locally running MySQL/MariaDB instance. Consider using a different host port (e.g.,13306:3306) or making it configurable.Suggested change
ports: - - "3306:3306" + - "13306:3306".devcontainer/Dockerfile (1)
9-10: Consider usingapt-getinstead ofaptin Dockerfiles.
aptis designed for interactive use and may produce warnings in non-interactive contexts.apt-getis the recommended command for scripts and Dockerfiles.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.devcontainer/Dockerfile.devcontainer/devcontainer.json.devcontainer/docker-compose.yamlCONTRIBUTING.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
CONTRIBUTING.md
43-43: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (6)
.devcontainer/docker-compose.yaml (1)
1-10: LGTM - Dev service is well configured.Good practices applied:
init: truefor proper signal handling,:cachedvolume option for better macOS performance, andsleep infinityto keep the container running for interactive use.CONTRIBUTING.md (1)
19-39: Clear and helpful documentation for the dev container workflow.The instructions cover both IDE-native and Docker Compose usage paths well, with practical examples.
.devcontainer/Dockerfile (3)
44-47: Good practice: cleaning apt cache to reduce image size.This properly removes cached packages after installation.
52-53: Note: Permissive git safe.directory setting.Setting
safe.directoryto'*'disables ownership checks for all directories. This is acceptable for a dev container but should not be replicated in production or CI environments.
49-50: The external hash.h URL is currently accessible.The GitHub raw URL for mysql-server/5.7/include/hash.h returns HTTP 200, confirming it's valid. For a dev container, the current approach is acceptable, though adding a checksum verification (via sha256sum or similar) would strengthen the supply chain posture if desired.
.devcontainer/devcontainer.json (1)
1-15: LGTM - Well-structured devcontainer configuration.The configuration correctly references the Docker Compose file and service, sets the appropriate workspace folder matching the volume mount, and includes relevant C++ development extensions.
| If you prefer to set up your environment manually, the required packages for building are listed in the Dockerfiles at: https://github.com/ProxySQL/docker-images/tree/main/build-images | ||
|
|
||
| For example, to see packages needed for Ubuntu 24.04, check: https://github.com/ProxySQL/docker-images/blob/main/build-images/build-ubuntu24/Dockerfile |
There was a problem hiding this comment.
Convert bare URL to markdown link for consistency.
Line 43 uses a bare URL while line 45 uses proper markdown link syntax. Per markdownlint (MD034), bare URLs should be avoided.
Suggested fix
-If you prefer to set up your environment manually, the required packages for building are listed in the Dockerfiles at: https://github.com/ProxySQL/docker-images/tree/main/build-images
+If you prefer to set up your environment manually, the required packages for building are listed in the Dockerfiles at: [ProxySQL/docker-images/build-images](https://github.com/ProxySQL/docker-images/tree/main/build-images)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| If you prefer to set up your environment manually, the required packages for building are listed in the Dockerfiles at: https://github.com/ProxySQL/docker-images/tree/main/build-images | |
| For example, to see packages needed for Ubuntu 24.04, check: https://github.com/ProxySQL/docker-images/blob/main/build-images/build-ubuntu24/Dockerfile | |
| If you prefer to set up your environment manually, the required packages for building are listed in the Dockerfiles at: [ProxySQL/docker-images/build-images](https://github.com/ProxySQL/docker-images/tree/main/build-images) | |
| For example, to see packages needed for Ubuntu 24.04, check: https://github.com/ProxySQL/docker-images/blob/main/build-images/build-ubuntu24/Dockerfile |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
43-43: Bare URL used
(MD034, no-bare-urls)
45-45: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In @CONTRIBUTING.md around lines 43 - 45, Replace the bare URL on the first
sentence with a markdown link to match the style used on the next line: change
the plain https://github.com/ProxySQL/docker-images/tree/main/build-images to a
markdown link like [ProxySQL docker-images
build-images](https://github.com/ProxySQL/docker-images/tree/main/build-images)
in CONTRIBUTING.md so both references use consistent markdown link syntax and
satisfy markdownlint MD034.
There was a problem hiding this comment.
This is a valid style nitpick. However, the existing CONTRIBUTING.md (before our changes) already had bare URLs in this section - we just moved them under a new "Manual Setup" heading.
There was a problem hiding this comment.
@rophy, that's a fair point! Since these bare URLs pre-existed your changes and you're just reorganizing the content, it makes sense that you wouldn't want to mix style fixes with functional changes in this PR.
If you'd like to address the markdownlint warnings, it could be done in a separate cleanup PR, but it's certainly not required for this dev container addition.
1 participant
Add IDE compatible dev container setup for
easier local development and testing.
Files added:
Usage with IDE: https://containers.dev/
Usage with docker compose:
The MariaDB service is available at hostname "mariadb" for testing.
Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.