Skip to content

#552 Move signal handling from queue component to controller#553

Open
snewer wants to merge 2 commits intoyiisoft:masterfrom
snewer:552-amqp-signal-registration
Open

#552 Move signal handling from queue component to controller#553
snewer wants to merge 2 commits intoyiisoft:masterfrom
snewer:552-amqp-signal-registration

Conversation

@snewer
Copy link
Copy Markdown

@snewer snewer commented Mar 3, 2026

Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Tests pass? ✔️
Fixed issues #552

Copy link
Copy Markdown
Member

@terabytesoftw terabytesoftw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"php": ">=8.3",

@snewer
Copy link
Copy Markdown
Author

snewer commented Mar 3, 2026

"php": ">=8.3",

I just moved the code, but yes, I agree it’s unnecessary here, so I removed it

@terabytesoftw
Copy link
Copy Markdown
Member

"php": ">=8.3",

I just moved the code, but yes, I agree it’s unnecessary here, so I removed it

The PHP 7.1 code is also unnecessary.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 4, 2026

Codecov Report

❌ Patch coverage is 0% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.98%. Comparing base (5be821e) to head (b1181f8).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/drivers/amqp_interop/Command.php 0.00% 14 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #553      +/-   ##
============================================
- Coverage     46.54%   45.98%   -0.57%     
  Complexity      506      506              
============================================
  Files            44       44              
  Lines          1579     1581       +2     
============================================
- Hits            735      727       -8     
- Misses          844      854      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@terabytesoftw terabytesoftw requested a review from s1lver March 4, 2026 07:34
@terabytesoftw terabytesoftw added the status:code review The pull request needs review. label Mar 4, 2026
@samdark samdark requested a review from Copilot March 5, 2026 12:21
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses #552 by preventing the AMQP Interop queue component from overriding process-wide UNIX signal handlers during application bootstrap, moving that behavior into the AMQP Interop console command lifecycle instead.

Changes:

  • Removed pcntl signal handler registration from amqp_interop\Queue::init() to avoid global side effects at bootstrap.
  • Added signal handler registration to amqp_interop\Command::init() so it only applies when the AMQP Interop queue command runs.
  • Documented the change in CHANGELOG.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/drivers/amqp_interop/Queue.php Stops registering signal handlers during component bootstrap to avoid impacting unrelated console commands.
src/drivers/amqp_interop/Command.php Registers signal handlers at command init time so behavior is scoped to the AMQP Interop CLI command execution.
CHANGELOG.md Adds an entry describing the signal-handling relocation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/drivers/amqp_interop/Command.php
Comment thread CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:code review The pull request needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants