Skip to content
This repository has been archived by the owner on May 2, 2024. It is now read-only.

Added shutdown_hook_priority to rabbitmq_server #49

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

stijnjanmaat
Copy link

@stijnjanmaat stijnjanmaat commented Feb 19, 2020

Description of the Change

When you have multiple actions taking place on the wp hook shutdown, you want to be able to postpone the shutdown of the Connection to RabbtiMQ. That's why I expanded the $rabbitmq_server config with shutdown_hook_priority.

Alternate Designs

Thought about setting the hook hard coded to PHP_INT_MAX but making it configurable is more flexible.

Benefits

You have more finegrained control of when the Connection is destroyed.

Possible Drawbacks

Maybe a bit more complex, but default behavior is unchanged.

Verification Process

I implemented fork in my project and it worked.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Changelog Entry

@jeffpaul jeffpaul requested a review from tlovett1 February 19, 2020 16:31
@jeffpaul jeffpaul added this to the 4.2.0 milestone Feb 19, 2020
@jeffpaul
Copy link
Member

@stijnjanmaat welcome to WP Minions and thanks for the feedback and PR. Some of our team is traveling for conferences and vacations right now but I'll get this to them when they're back and let you know if there are any questions/concerns from their review. Thanks again!

@jeffpaul jeffpaul requested a review from dinhtungdu March 21, 2020 22:37
@stijnjanmaat
Copy link
Author

Hi @jeffpaul, did you get a chance to look at this request?

@jeffpaul
Copy link
Member

@stijnjanmaat not as of yet, no, but with WP 5.4 coming next week we're working in parallel to test compatibility there so expect some closer review of this PR in the near future. I appreciate your patience, thanks!

@jeffpaul jeffpaul changed the base branch from master to develop May 25, 2021 19:25
@dkotter dkotter removed the request for review from dinhtungdu August 23, 2022 22:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants