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

Asyncio server will set serving=False if it fails to start #715

Closed

Conversation

couling
Copy link
Contributor

@couling couling commented Feb 17, 2022

Asyncio server should notify that it is NOT serving if the server fails to start.

At this time, the server has no server.start_serving() method only a server.serve_forever() method. This means the only way to detect if the server is ready to receive connections await server.serving() with defer_start=False.

This is particularly useful when using the modbus server in a unit test.

import asyncio

async def use_server_for_test(context):
    server = ModbusTcpServer(context)
    server_task = asyncio.get_running_loop().create_task(server.serve_forever())
    try:
        assert await server.serving
        # Test a client by connecting to this server.
        test_my_client(server.server.sockets[0].getsockname())
    finally:
        server_task.cancel()

At the moment the above code would just hang and I can see no other future that will return both when the server starts and if it fails to start.

This PR then sets the serving to False if the server fails to startup.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@janiversen janiversen force-pushed the notify-server-failed-to-start branch from 02f46ae to 8669ac2 Compare April 3, 2022 16:13
@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 3, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
1.7% 1.7% Duplication

@janiversen
Copy link
Collaborator

This code is merged into dev.

@janiversen janiversen closed this Apr 4, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
@couling couling deleted the notify-server-failed-to-start branch May 16, 2023 22:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants