8000 fix: task subprocesses by loks0n · Pull Request #11040 · appwrite/appwrite · GitHub
[go: up one dir, main page]
Skip to content

fix: task subprocesses#11040

Merged
loks0n merged 1 commit into1.8.xfrom
fix-task-subprocesses
Dec 30, 2025
Merged

fix: task subprocesses#11040
loks0n merged 1 commit into1.8.xfrom
fix-task-subprocesses

Conversation

@loks0n
Copy link
Member
@loks0n loks0n commented Dec 29, 2025 •
edited
Loading

Use exec to replace the shell process with PHP, ensuring signals (SIGTERM, SIGINT, etc.) are delivered directly to the PHP process instead of the wrapper shell. Also quote $@ properly for argument handling.

Before:
php /usr/src/code/app/cli.php doctor $@

After:
exec php /usr/src/code/app/cli.php doctor "$@"

@loks0n loks0n requested a review from levivannoort December 29, 2025 21:55
@github-actions
Copy link
github-actions bot commented Dec 29, 2025 •
edited
Loading

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
libpng 1.6.51-r0 CVE-2025-66293 HIGH
libpng-dev 1.6.51-r0 CVE-2025-66293 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

@coderabbitai
Copy link
Contributor
coderabbitai bot commented Dec 29, 2025 •
edited
Loading
📝 Walkthrough

Walkthrough

This pull request updates many shell scripts in the bin/ directory to replace direct PHP (and phpunit) invocations with exec-wrapped calls and to quote argument expansion. Instances of forms like php /usr/src/code/app/cli.php <cmd> $@ are changed to exec php /usr/src/code/app/cli.php <cmd> "$@", altering process replacement semantics and preserving argument boundaries via "$@" across the affected launchers.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: task subprocesses' directly summarizes the main change across all modified files—replacing shell invocations with exec to ensure signals are delivered directly to PHP processes.
Description check ✅ Passed The description explains the purpose and motivation for the changes, providing a clear before/after example that accurately reflects the modifications made throughout the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b7cced7 and 55fbd77.

📒 Files selected for processing (33)
  • bin/doctor
  • bin/install
  • bin/interval
  • bin/maintenance
  • bin/migrate
  • bin/queue-count-failed
  • bin/queue-count-processing
  • bin/queue-count-success
  • bin/queue-retry
  • bin/realtime
  • bin/schedule-executions
  • bin/schedule-functions
  • bin/schedule-messages
  • bin/screenshot
  • bin/sdks
  • bin/specs
  • bin/ssl
  • bin/stats-resources
  • bin/test
  • bin/upgrade
  • bin/vars
  • bin/worker-audits
  • bin/worker-builds
  • bin/worker-certificates
  • bin/worker-databases
  • bin/worker-deletes
  • bin/worker-functions
  • bin/worker-mails
  • bin/worker-messaging
  • bin/worker-migrations
  • bin/worker-stats-resources
  • bin/worker-stats-usage
  • bin/worker-webhooks
🚧 Files skipped from review as they are similar to previous changes (23)
  • bin/worker-audits
  • bin/migrate
  • bin/worker-mails
  • bin/stats-resources
  • bin/doctor
  • bin/vars
  • bin/maintenance
  • bin/schedule-messages
  • bin/worker-stats-usage
  • bin/screenshot
  • bin/worker-deletes
  • bin/specs
  • bin/ssl
  • bin/queue-retry
  • bin/queue-count-failed
  • bin/worker-messaging
  • bin/worker-stats-resources
  • bin/schedule-executions
  • bin/queue-count-processing
  • bin/realtime
  • bin/queue-count-success
  • bin/interval
  • bin/install
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
  • GitHub Check: E2E Service Test (Avatars)
  • GitHub Check: E2E Service Test (Webhooks)
  • GitHub Check: E2E Service Test (Users)
  • GitHub Check: E2E Service Test (Projects)
  • GitHub Check: E2E Service Test (Proxy)
  • GitHub Check: E2E Service Test (FunctionsSchedule)
  • GitHub Check: E2E Service Test (Storage)
  • GitHub Check: E2E Service Test (Locale)
  • GitHub Check: E2E Service Test (Functions)
  • GitHub Check: E2E Service Test (Realtime)
  • GitHub Check: E2E Service Test (Teams)
  • GitHub Check: E2E Service Test (Messaging)
  • GitHub Check: E2E Service Test (Sites)
  • GitHub Check: E2E Service Test (Databases/Legacy)
  • GitHub Check: E2E Service Test (Databases/TablesDB)
  • GitHub Check: E2E Service Test (Site Screenshots)
  • GitHub Check: E2E Service Test (Dev Keys)
  • GitHub Check: E2E General Test
  • GitHub Check: Unit Test
  • GitHub Check: scan
🔇 Additional comments (10)
bin/worker-webhooks (1)

3-3: Excellent improvements for signal handling and argument preservation.

The use of exec ensures signals are delivered directly to the PHP process, and "$@" properly preserves argument boundaries.

bin/upgrade (1)

3-3: LGTM!

The change correctly implements process replacement with exec and proper argument quoting with "$@".

bin/worker-functions (1)

3-3: LGTM!

Correctly applies exec for process replacement and "$@" for argument preservation.

bin/schedule-functions (1)

3-3: LGTM!

The implementation correctly uses exec and "$@" for proper process management and argument handling.

bin/worker-builds (1)

3-3: LGTM!

Properly implements process replacement and argument quoting.

bin/sdks (1)

3-3: LGTM!

The change correctly applies the exec and "$@" pattern for improved signal handling and argument preservation.

bin/worker-certificates (1)

3-3: LGTM!

Correctly implements the exec-based invocation with proper argument quoting.

bin/worker-migrations (1)

3-3: LGTM!

The change properly uses exec for process replacement and "$@" for argument preservation, consistent with the rest of the PR.

bin/worker-databases (1)

3-3: LGTM! Proper signal handling and argument quoting.

The use of exec ensures signals reach the PHP process directly, and "$@" correctly preserves argument boundaries with spaces and special characters.

bin/test (1)

3-3: LGTM! Correct implementation for test wrapper.

The exec and "$@" changes ensure proper signal handling and argument preservation for the phpunit wrapper, consistent with the PR objectives.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
github-actions bot commented Dec 29, 2025 •
edited
Loading

✨ Benchmark results

  • Requests per second: 1,112
  • Requests with 200 status code: 200,133
  • P99 latency: 0.174206705

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,112 1,237
200 200,133 222,652
P99 0.174206705 0.16917297

@loks0n loks0n force-pushed the fix-task-subprocesses branch from b7cced7 to 55fbd77 Compare December 30, 2025 09:13
@loks0n loks0n merged commit 6499f43 into 1.8.x Dec 30, 2025
40 of 41 checks passed
This was referenced Feb 3, 2026
Merged
Merged
Merged
Merged
Merged
This was referenced Feb 19, 2026
Merged
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@levivannoort levivannoort levivannoort approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@loks0n @levivannoort
0