feat: add direnv/devbox configurations and fix tests#759
Conversation
WalkthroughSeveral files were added or updated to enhance the development environment setup and testing. New configuration files for devbox, Redis, and process management were introduced. Documentation was updated to remove Docker references. Test code was refactored for clarity and improved mock handling, and ignore patterns were broadened for coverage files. Changes
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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.
Actionable comments posted: 2
🧹 Nitpick comments (9)
devbox.json (1)
2-2: Consider using a more stable schema version or latest.The schema URL points to a specific version (0.14.0) which may become outdated. Consider using the latest schema or a more recent stable version.
- "$schema": "https://raw.githubusercontent.com/jetify-com/devbox/0.14.0/.schema/devbox.schema.json", + "$schema": "https://raw.githubusercontent.com/jetify-com/devbox/main/.schema/devbox.schema.json",tests/ext/test_ext_watchdog.py (2)
36-39: Consider using context managers for file operations.The file operations could be improved by using context managers to ensure proper resource cleanup.
- f = open(file_path, 'w') - f.write('test data') - f.close() + with open(file_path, 'w') as f: + f.write('test data')🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 36-36: Consider using 'with' for resource-allocating operations
(R1732)
84-87: Apply context manager pattern consistently.Similar to the previous suggestion, use context managers for file operations throughout the test file.
- f = open(fs.join(tmp.dir, 'test.file'), 'w') - f.write('test data') - f.close() + with open(fs.join(tmp.dir, 'test.file'), 'w') as f: + f.write('test data')Also applies to: 142-145
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 84-84: Consider using 'with' for resource-allocating operations
(R1732)
.envrc (3)
4-4: Ensure devbox CLI availability
Consider adding a sanity check to fail fast ifdevboxis not installed, giving clearer guidance:+if ! command -v devbox >/dev/null; then + echo "error: devbox CLI not found. Please install devbox." >&2 + exit 1 +fi eval "$(devbox generate direnv --print-envrc)"
11-14: Add missing port exports for consistency
You export hosts but not ports for Redis/Memcached. It’s clearer to include defaults:export SMTP_HOST=localhost export SMTP_PORT=1025 +export MEMCACHED_PORT=11211 export MEMCACHED_HOST=localhost +export REDIS_PORT=6379 export REDIS_HOST=localhost
15-15: Ensure final newline
Add a trailing newline to the end of the file to satisfy POSIX conventions.process-compose.yml (2)
1-1: Update header comment for accuracy
The comment references "starting django" but this file only configures background services. Consider:-# Process compose for starting django +# Process compose for background services (memcached, mailpit)
2-2: Standardize YAML indentation
For readability, align with a 2-space indentation style:-version: "0.5" +version: "0.5"(and shift nested keys accordingly)
devbox.d/redis/redis.conf (1)
218-220: Disable RDB persistence in dev
You may not need snapshotting in local dev. Comment outsavedirectives or set them to empty:-save 900 1 -save 300 10 -save 60 10000 +save ""
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
devbox.lockis excluded by!**/*.lockpdm.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.envrc(1 hunks).gitignore(1 hunks)CLAUDE.md(1 hunks)devbox.d/redis/redis.conf(1 hunks)devbox.json(1 hunks)process-compose.yml(1 hunks)tests/ext/test_ext_daemon.py(1 hunks)tests/ext/test_ext_watchdog.py(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/ext/test_ext_daemon.py (4)
cement/core/foundation.py (4)
TestApp(1760-1784)argv(846-848)run(936-964)close(1015-1050)cement/ext/ext_daemon.py (3)
daemonize(105-173)daemonize(176-211)cleanup(241-256)tests/conftest.py (1)
tmp(10-16)cement/utils/shell.py (1)
spawn_process(183-232)
🪛 Pylint (3.3.7)
tests/ext/test_ext_daemon.py
[error] 87-87: Instance of 'TestApp' has no 'daemonize' member
(E1101)
tests/ext/test_ext_watchdog.py
[error] 31-31: Instance of 'WatchdogApp' has no 'watchdog' member
(E1101)
[error] 56-56: Method 'on_any_event' has no 'call_count' member
(E1101)
[refactor] 36-36: Consider using 'with' for resource-allocating operations
(R1732)
[error] 82-82: Method 'on_any_event' has no 'reset_mock' member
(E1101)
[error] 111-111: Method 'on_any_event' has no 'call_count' member
(E1101)
[refactor] 84-84: Consider using 'with' for resource-allocating operations
(R1732)
[error] 139-139: Instance of 'WatchdogApp' has no 'watchdog' member
(E1101)
[error] 163-163: Method 'on_any_event' has no 'call_count' member
(E1101)
[refactor] 142-142: Consider using 'with' for resource-allocating operations
(R1732)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (11)
devbox.json (1)
4-4:Details
✅ Verification successful
Verify Python 3.13 availability and stability.
Python 3.13 is very recent. Ensure it's stable and widely available across development environments, or consider using a more established version like 3.11 or 3.12.
🌐 Web query:
Is Python 3.13 stable and recommended for production use?💡 Result:
As of June 10, 2025, Python 3.13.4 is the latest stable release, and it is recommended for production use. This version is currently receiving full bug-fix and security updates. Python 3.13 introduced several enhancements, including improved performance and new features like an experimental just-in-time (JIT) compiler and an optional free-threaded build mode that allows for better multi-core CPU utilization. (en.wikipedia.org)
Python 3.13 was released on October 7, 2024, and is scheduled to receive full support until May 2026, followed by security support until October 2029. (en.wikipedia.org)
Given its current support status and the introduction of new features, Python 3.13 is suitable for production environments.
Citations:
- 1: https://en.wikipedia.org/wiki/Python_%28programming_language%29?utm_source=openai
- 2: https://en.wikipedia.org/wiki/History_of_Python?utm_source=openai
Confirm Python 3.13 Stability
As of June 2025, Python 3.13.4 is the latest stable release and is recommended for production use. It receives full bug-fix and security updates until May 2026 (with security support through October 2029) and introduces performance enhancements (e.g., an experimental JIT compiler and free-threaded build mode). Specifying
"python": "3.13"in devbox.json is appropriate and requires no changes.tests/ext/test_ext_daemon.py (2)
80-92: LGTM! Good refactoring to extract daemon test logic.The extraction of the daemon target function improves code organization and reusability. The proper cleanup handling in the
finallyblock is also well implemented.Note: The static analysis error about
TestAppmissingdaemonizemember is a false positive - the daemon extension dynamically adds this method to the app instance.🧰 Tools
🪛 Pylint (3.3.7)
[error] 87-87: Instance of 'TestApp' has no 'daemonize' member
(E1101)
99-99: Good use of subprocess isolation for daemon testing.Using
spawn_processwith the extracted helper function properly isolates the daemon test execution, which is essential for testing daemonization without affecting the test runner.tests/ext/test_ext_watchdog.py (2)
24-28: Excellent mock handling improvements.The addition of explicit mock reset logic before and after tests prevents test interference and ensures clean state. The defensive checking for existing mock attributes is also well implemented.
Also applies to: 57-61
29-56: Good use of try/finally for robust test execution.Wrapping the test logic in try/finally blocks ensures proper mock cleanup even if tests fail. The relaxed assertion counts appropriately handle platform differences in filesystem events.
🧰 Tools
🪛 Pylint (3.3.7)
[error] 31-31: Instance of 'WatchdogApp' has no 'watchdog' member
(E1101)
[error] 56-56: Method 'on_any_event' has no 'call_count' member
(E1101)
[refactor] 36-36: Consider using 'with' for resource-allocating operations
(R1732)
.gitignore (1)
55-55: Good update to handle multiple coverage file formats.Broadening the coverage ignore pattern from
.coverageto.coverage*properly handles various coverage file formats and naming patterns that different tools and environments might generate.CLAUDE.md (1)
78-78: Good documentation cleanup aligned with new development environment.Removing the Docker development environment references properly aligns the documentation with the new devbox/direnv-based approach. The focus on PDM-based local development is more consistent with the simplified setup goals.
.envrc (1)
9-9: Confirmsource_env_if_existsis defined
Verify that the function comes from the generated envrc or direnv; otherwise consider replacing with the built-indotenvhelper:- source_env_if_exists .envrc.local + dotenvprocess-compose.yml (1)
18-19: Verify TLS certificate paths
The mailpit cert and key paths (docker/mailpit/dev-cert.pem) might not exist in your devbox layout. Confirm these files are present or update the paths.devbox.d/redis/redis.conf (2)
263-263: Validate working directory path
You setdir .devbox/virtenv/redis/. Make sure Redis can write here in the devbox environment—adjust to the actual virtual-env path if necessary.
88-88: Confirm secure bind and protected mode
Binding to127.0.0.1is good. Also verifyprotected-mode yesremains enabled to prevent unintended exposure.
| "shell": { | ||
| "init_hook": [], | ||
| "scripts": { | ||
| "test": ["echo \"Error: no test specified\" && exit 1"] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace placeholder test script with actual test command.
The test script is a placeholder that always fails. Consider replacing it with the actual test command from the project.
- "test": ["echo \"Error: no test specified\" && exit 1"]
+ "test": ["make test"]📝 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.
| "test": ["echo \"Error: no test specified\" && exit 1"] | |
| "test": ["make test"] |
🤖 Prompt for AI Agents
In devbox.json at line 21, the test script currently uses a placeholder command
that always fails. Replace this placeholder with the actual test command used by
the project, such as the appropriate script or command that runs the project's
tests, to ensure tests can be executed properly.
| # Redis configuration file example. | ||
| # | ||
| # Note that in order to read the configuration file, Redis must be | ||
| # started with the file path as first argument: | ||
| # | ||
| # ./redis-server /path/to/redis.conf | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce duplication by including the system default
Maintaining the entire default config is heavy. Instead, include the standard file and override only needed options:
-# Redis configuration file example.
+# Include system config and override for dev
+include /etc/redis/redis.conf📝 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.
| # Redis configuration file example. | |
| # | |
| # Note that in order to read the configuration file, Redis must be | |
| # started with the file path as first argument: | |
| # | |
| # ./redis-server /path/to/redis.conf | |
| # Include system config and override for dev | |
| include /etc/redis/redis.conf | |
| # | |
| # Note that in order to read the configuration file, Redis must be | |
| # started with the file path as first argument: | |
| # | |
| # ./redis-server /path/to/redis.conf |
🤖 Prompt for AI Agents
In devbox.d/redis/redis.conf lines 1 to 7, the configuration duplicates the
entire default Redis config, which is heavy to maintain. Replace the full config
with an include directive to the system default Redis config file, then specify
only the options you need to override below it. This reduces duplication and
simplifies maintenance.
What:
Add configurations for direnv/devbox as an alternative to the (heavy) docker compose setup currently. Running tests on macos had issues so also fixes some mocks.
Why:
Experimenting with Claude Code, and not being able to execute commands like
make testnatively is problematic. Also interested to simplify the development environment which works, but is heavy/complex for some (not familiar with having to exec into containers to dev/test).Summary by CodeRabbit