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

Added support to Multiplexer and Manager to terminate durable workers #2913

Closed
wants to merge 2 commits into from

Conversation

john-woolley
Copy link

@john-woolley john-woolley commented Feb 10, 2024

I had a usecase for this in a project I'm working on and I thought it might be useful. Happy to make any changes to be more in-line with the existing codebase or planned trajectory.

@john-woolley john-woolley requested a review from a team as a code owner February 10, 2024 02:10
Copy link

codecov bot commented Feb 10, 2024

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (acb29c9) 88.039% compared to head (0f22249) 87.892%.

❗ Current head 0f22249 differs from pull request most recent head 062adaa. Consider uploading reports for the commit 062adaa to get more accurate results

Files Patch % Lines
sanic/worker/manager.py 31.250% 8 Missing and 3 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##              main     #2913       +/-   ##
=============================================
- Coverage   88.039%   87.892%   -0.148%     
=============================================
  Files           94        94               
  Lines         7433      7450       +17     
  Branches      1283      1287        +4     
=============================================
+ Hits          6544      6548        +4     
- Misses         622       632       +10     
- Partials       267       270        +3     

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

sanic/worker/manager.py Outdated Show resolved Hide resolved
@john-woolley
Copy link
Author

john-woolley commented Feb 14, 2024

________________________ test_terminate_durable_worker _________________________

manager = <sanic.worker.manager.WorkerManager object at 0x7f4f58e75190>
caplog = <_pytest.logging.LogCaptureFixture object at 0x7f4f58f30910>

    def test_terminate_durable_worker(manager: WorkerManager, caplog):
        worker = manager.manage("TEST", fake_serve, kwargs={})
    
        assert "Sanic-TEST-0" in worker.worker_state
        assert len(manager.transient) == 1
        assert len(manager.durable) == 1
    
        manager.terminate_durable_worker("WRONG")
        message = "Durable worker termination failed because WRONG not found."
    
        assert "Sanic-TEST-0" in worker.worker_state
        assert len(manager.transient) == 1
        assert len(manager.durable) == 1
>       assert ("sanic.error", 40, message) in caplog.record_tuples
E       AssertionError: assert ('sanic.error', 40, 'Durable worker termination failed because WRONG not found.') in [('sanic.error', 40, 'Durable worker termination failed (None not found).')]
E        +  where [('sanic.error', 40, 'Durable worker termination failed (None not found).')] = <_pytest.logging.LogCaptureFixture object at 0x7f4f58f30910>.record_tuples

/home/runner/work/sanic/sanic/tests/worker/test_manager.py:433: AssertionError```

Closing this for now until I figure this out.

worker = self.durable.get(ident)
if not worker:
error_logger.error(
"Durable worker termination failed (%s not found).", worker
Copy link
Member

Choose a reason for hiding this comment

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

Should be , ident; not , worker. That seems to be what's making the test fail.

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.

None yet

3 participants