Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Unsubscribe from transport on closing connection #12

Merged
merged 1 commit into from
Apr 22, 2022

Conversation

bpolaszek
Copy link
Owner

Attempt to fix #11

@codecov
Copy link

codecov bot commented Apr 14, 2022

Codecov Report

Merging #12 (7133e69) into main (e5338ad) will not change coverage.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##                main       #12   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity       130       133    +3     
===========================================
  Files             23        23           
  Lines            345       353    +8     
===========================================
+ Hits             345       353    +8     
Impacted Files Coverage Δ
src/Hub/Controller/SubscribeController.php 100.00% <100.00%> (ø)
src/Hub/Hub.php 100.00% <100.00%> (ø)
src/Hub/Transport/PHP/PHPTransport.php 100.00% <100.00%> (ø)
src/Hub/Transport/Redis/RedisTransport.php 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5338ad...7133e69. Read the comment docs.

@progmancod
Copy link

progmancod commented Apr 14, 2022

I thought the correction idea was excellent. I will test it immediately. It will be possible to implement the metrics system in the same way. I will do a PR with the implementations of metrics and heartbeat.

@bpolaszek
Copy link
Owner Author

Hello @progmancod, just pinging 🙂
Did you get time to test this fix?

@progmancod
Copy link

Sorry for the delay, it's working very well. I will send you soon PRs for Docker, metrics and hearbeat.

@bpolaszek
Copy link
Owner Author

Alright, let's merge this! 🎸

Thank you very much for your support! 👍

@bpolaszek bpolaszek merged commit 3cef130 into main Apr 22, 2022
@bpolaszek bpolaszek deleted the fix/flush-subscribers branch April 22, 2022 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance degradation over time
2 participants