Skip to content

Commit

Permalink
python/aqmp: rename 'accept()' to 'start_server_and_accept()'
Browse files Browse the repository at this point in the history
Previously, I had a method named "accept()" that under-the-hood calls
bind(2), listen(2) *and* accept(2). I meant this as a simplification and
counterpart to the one-shot "connect()" method.

This is confusing to readers who expect accept() to mean *just*
accept(2). Since I need to split apart the "accept()" method into
multiple methods anyway (one of which strongly resembling accept(2)), it
feels pertinent to rename this method *now*.

Rename this all-in-one method "start_server_and_accept()" instead.

Signed-off-by: John Snow <jsnow@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20220225205948.3693480-3-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
  • Loading branch information
jnsnow committed Mar 7, 2022
1 parent 40196c2 commit 0ba4e76
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 15 deletions.
2 changes: 1 addition & 1 deletion python/qemu/aqmp/legacy.py
Expand Up @@ -91,7 +91,7 @@ def accept(self, timeout: Optional[float] = 15.0) -> QMPMessage:
self._aqmp.negotiate = True

self._sync(
self._aqmp.accept(self._address),
self._aqmp.start_server_and_accept(self._address),
timeout
)

Expand Down
6 changes: 4 additions & 2 deletions python/qemu/aqmp/protocol.py
Expand Up @@ -265,8 +265,10 @@ async def runstate_changed(self) -> Runstate:

@upper_half
@require(Runstate.IDLE)
async def accept(self, address: SocketAddrT,
ssl: Optional[SSLContext] = None) -> None:
async def start_server_and_accept(
self, address: SocketAddrT,
ssl: Optional[SSLContext] = None
) -> None:
"""
Accept a connection and begin processing message queues.
Expand Down
24 changes: 12 additions & 12 deletions python/tests/protocol.py
Expand Up @@ -413,14 +413,14 @@ async def _bad_connection(self, family: str):
assert family in ('INET', 'UNIX')

if family == 'INET':
await self.proto.accept(('example.com', 1))
await self.proto.start_server_and_accept(('example.com', 1))
elif family == 'UNIX':
await self.proto.accept('/dev/null')
await self.proto.start_server_and_accept('/dev/null')

async def _hanging_connection(self):
with TemporaryDirectory(suffix='.aqmp') as tmpdir:
sock = os.path.join(tmpdir, type(self.proto).__name__ + ".sock")
await self.proto.accept(sock)
await self.proto.start_server_and_accept(sock)


class FakeSession(TestBase):
Expand Down Expand Up @@ -449,13 +449,13 @@ async def testFakeConnect(self):
@TestBase.async_test
async def testFakeAccept(self):
"""Test the full state lifecycle (via accept) with a no-op session."""
await self.proto.accept('/not/a/real/path')
await self.proto.start_server_and_accept('/not/a/real/path')
self.assertEqual(self.proto.runstate, Runstate.RUNNING)

@TestBase.async_test
async def testFakeRecv(self):
"""Test receiving a fake/null message."""
await self.proto.accept('/not/a/real/path')
await self.proto.start_server_and_accept('/not/a/real/path')

logname = self.proto.logger.name
with self.assertLogs(logname, level='DEBUG') as context:
Expand All @@ -471,7 +471,7 @@ async def testFakeRecv(self):
@TestBase.async_test
async def testFakeSend(self):
"""Test sending a fake/null message."""
await self.proto.accept('/not/a/real/path')
await self.proto.start_server_and_accept('/not/a/real/path')

logname = self.proto.logger.name
with self.assertLogs(logname, level='DEBUG') as context:
Expand All @@ -493,7 +493,7 @@ async def _prod_session_api(
):
with self.assertRaises(StateError) as context:
if accept:
await self.proto.accept('/not/a/real/path')
await self.proto.start_server_and_accept('/not/a/real/path')
else:
await self.proto.connect('/not/a/real/path')

Expand All @@ -504,7 +504,7 @@ async def _prod_session_api(
@TestBase.async_test
async def testAcceptRequireRunning(self):
"""Test that accept() cannot be called when Runstate=RUNNING"""
await self.proto.accept('/not/a/real/path')
await self.proto.start_server_and_accept('/not/a/real/path')

await self._prod_session_api(
Runstate.RUNNING,
Expand All @@ -515,7 +515,7 @@ async def testAcceptRequireRunning(self):
@TestBase.async_test
async def testConnectRequireRunning(self):
"""Test that connect() cannot be called when Runstate=RUNNING"""
await self.proto.accept('/not/a/real/path')
await self.proto.start_server_and_accept('/not/a/real/path')

await self._prod_session_api(
Runstate.RUNNING,
Expand All @@ -526,7 +526,7 @@ async def testConnectRequireRunning(self):
@TestBase.async_test
async def testAcceptRequireDisconnecting(self):
"""Test that accept() cannot be called when Runstate=DISCONNECTING"""
await self.proto.accept('/not/a/real/path')
await self.proto.start_server_and_accept('/not/a/real/path')

# Cheat: force a disconnect.
await self.proto.simulate_disconnect()
Expand All @@ -541,7 +541,7 @@ async def testAcceptRequireDisconnecting(self):
@TestBase.async_test
async def testConnectRequireDisconnecting(self):
"""Test that connect() cannot be called when Runstate=DISCONNECTING"""
await self.proto.accept('/not/a/real/path')
await self.proto.start_server_and_accept('/not/a/real/path')

# Cheat: force a disconnect.
await self.proto.simulate_disconnect()
Expand Down Expand Up @@ -576,7 +576,7 @@ async def _asyncTearDown(self):
async def testSmoke(self):
with TemporaryDirectory(suffix='.aqmp') as tmpdir:
sock = os.path.join(tmpdir, type(self.proto).__name__ + ".sock")
server_task = create_task(self.server.accept(sock))
server_task = create_task(self.server.start_server_and_accept(sock))

# give the server a chance to start listening [...]
await asyncio.sleep(0)
Expand Down

0 comments on commit 0ba4e76

Please sign in to comment.