chore: docker/postgres tuning, spp CLI enhancements, pre-commit + agent docs (re-land from #76)#278
Conversation
…it + agent docs (from #76) Re-lands the infrastructure/tooling portion of PR #76, which was reverted wholesale in d38ff9d. Restores docker entrypoint/config, postgresql.conf, docker-compose tuning, spp CLI enhancements, pre-commit config, and agent docs (AGENTS.md, .agents skill) from the pre-revert merged state 8bf9a3a.
![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 several enhancements to the OpenSPP development environment, including comprehensive development and UI guidelines, pinned pre-commit dependencies, and PostgreSQL performance and logging configurations. Key updates to the spp CLI tool include stable port assignment based on project path hashes, a new misluzon demo profile, and a slow-queries command to monitor and analyze slow database queries. The review feedback highlights critical robustness improvements for the CLI, specifically handling potential ValueError exceptions when parsing port and query threshold inputs, and ensuring proper subprocess cleanup when streaming slow queries to prevent resource leaks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| slow_queries = getattr(args, "slow_queries", None) | ||
| if slow_queries is not None: | ||
| env["PG_LOG_MIN_DURATION"] = slow_queries | ||
| threshold = int(slow_queries) | ||
| if threshold == 0: | ||
| info("Slow query logging: all queries (threshold=0ms)") | ||
| else: | ||
| info(f"Slow query logging: threshold={threshold}ms") | ||
| info("View with: ./spp slow-queries or ./spp sq -f") |
There was a problem hiding this comment.
If --slow-queries is passed with a non-integer value (e.g., --slow-queries=abc), int(slow_queries) will raise a ValueError and crash the CLI with a traceback. Wrapping this in a try-except block allows the CLI to handle invalid inputs gracefully and exit with a clear error message.
| slow_queries = getattr(args, "slow_queries", None) | |
| if slow_queries is not None: | |
| env["PG_LOG_MIN_DURATION"] = slow_queries | |
| threshold = int(slow_queries) | |
| if threshold == 0: | |
| info("Slow query logging: all queries (threshold=0ms)") | |
| else: | |
| info(f"Slow query logging: threshold={threshold}ms") | |
| info("View with: ./spp slow-queries or ./spp sq -f") | |
| # Set slow query threshold if specified | |
| slow_queries = getattr(args, "slow_queries", None) | |
| if slow_queries is not None: | |
| try: | |
| threshold = int(slow_queries) | |
| env["PG_LOG_MIN_DURATION"] = str(threshold) | |
| if threshold == 0: | |
| info("Slow query logging: all queries (threshold=0ms)") | |
| else: | |
| info(f"Slow query logging: threshold={threshold}ms") | |
| info("View with: ./spp slow-queries or ./spp sq -f") | |
| except ValueError: | |
| error(f"Invalid slow-queries threshold '{slow_queries}'. Must be an integer.") | |
| sys.exit(1) |
| env_port = os.environ.get("ODOO_HOST_PORT") | ||
| if env_port and env_port != "0": | ||
| return int(env_port) | ||
|
|
||
| config_port = CONFIG.get("stable_port") | ||
| if config_port: | ||
| return int(config_port) |
There was a problem hiding this comment.
If ODOO_HOST_PORT or stable_port in the configuration is set to a non-integer value, calling int() on them will raise a ValueError and crash the CLI. Wrapping these in try-except blocks ensures that invalid values are handled gracefully, falling back to the default port generation logic.
| env_port = os.environ.get("ODOO_HOST_PORT") | |
| if env_port and env_port != "0": | |
| return int(env_port) | |
| config_port = CONFIG.get("stable_port") | |
| if config_port: | |
| return int(config_port) | |
| env_port = os.environ.get("ODOO_HOST_PORT") | |
| if env_port and env_port != "0": | |
| try: | |
| return int(env_port) | |
| except ValueError: | |
| warn(f"Invalid ODOO_HOST_PORT '{env_port}', ignoring.") | |
| config_port = CONFIG.get("stable_port") | |
| if config_port: | |
| try: | |
| return int(config_port) | |
| except ValueError: | |
| warn(f"Invalid stable_port '{config_port}' in config, ignoring.") |
| try: | ||
| proc = subprocess.Popen( | ||
| cmd, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.STDOUT, | ||
| text=True, | ||
| cwd=PROJECT_ROOT, | ||
| ) | ||
| for line in proc.stdout: | ||
| duration, statement = _parse_slow_query_line(line) | ||
| if duration is not None: | ||
| if threshold and duration < threshold: | ||
| continue | ||
| color_code = "31" if duration >= 1000 else "33" if duration >= 500 else "32" | ||
| print(f"{_color(color_code, f'{duration:>10.1f}ms')} {statement.strip()}") | ||
| except KeyboardInterrupt: | ||
| proc.terminate() | ||
| print() | ||
| return |
There was a problem hiding this comment.
If the subprocess is successfully created but an exception other than KeyboardInterrupt occurs, or if the loop exits normally, the process is not terminated or waited on. This can leak resources or leave zombie processes. Using a finally block ensures that the subprocess is always terminated and waited on properly.
proc = None
try:
proc = subprocess.Popen(
cmd,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
text=True,
cwd=PROJECT_ROOT,
)
for line in proc.stdout:
duration, statement = _parse_slow_query_line(line)
if duration is not None:
if threshold and duration < threshold:
continue
color_code = "31" if duration >= 1000 else "33" if duration >= 500 else "32"
print(f"{_color(color_code, f'{duration:>10.1f}ms')} {statement.strip()}")
except KeyboardInterrupt:
print()
finally:
if proc:
proc.terminate()
try:
proc.wait(timeout=5)
except subprocess.TimeoutExpired:
proc.kill()
proc.wait()![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return int(config_port) | ||
|
|
||
| path = path or str(PROJECT_ROOT) | ||
| digest = hashlib.md5(path.encode()).hexdigest() # nosec B324 -- not for security |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 19.0 #278 +/- ##
=======================================
Coverage 74.85% 74.86%
=======================================
Files 1093 1093
Lines 63735 63718 -17
=======================================
- Hits 47708 47701 -7
+ Misses 16027 16017 -10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
2 participants
Re-lands the infrastructure/tooling portion of reverted PR #76 (revert: #271). No Odoo module code.
Summary
docker/postgresql.conf(new) + entrypoint/odoo.conf template updates; docker-compose changes.sppCLI enhancements (+215 lines: slow-query tooling, demo profiles, test runner improvements)..pre-commit-config.yamladditions.AGENTS.mdand.agents/skills/openspp-ui/SKILL.mddocs.Verification
bash -n docker/entrypoint.shOK; docker-compose.yml parses as valid YAML;sppkeeps its executable bit. No module tests apply.