Skip to content

Commit 7d59e41

Browse files
authored
Rename name to unique_id for BackgroundService (#26)
Using `name` was confusing, as people didn't realize that it is used to actually identify one of multiple instances. Also people were very prone to use the class name as `name`, which is redundant as the class name is also included in `str` and `repr`. Using `unique_id` makes it more clear that this is used for identification in logs and that a default is better than passing a string that is not really unique (like the class name). Also use `hex()` instead of `str()` to build the default `unique_id`, as it a more compact representation of the id. Fixes #7.
2 parents c62020a + 6a48ed7 commit 7d59e41

File tree

2 files changed

+34
-27
lines changed

2 files changed

+34
-27
lines changed

src/frequenz/core/asyncio.py

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ class BackgroundService(abc.ABC):
7373
import asyncio
7474
7575
class Clock(BackgroundService):
76-
def __init__(self, resolution_s: float, *, name: str | None = None) -> None:
77-
super().__init__(name=name)
76+
def __init__(self, resolution_s: float, *, unique_id: str | None = None) -> None:
77+
super().__init__(unique_id=unique_id)
7878
self._resolution_s = resolution_s
7979
8080
def start(self) -> None:
@@ -100,28 +100,32 @@ async def main() -> None:
100100
```
101101
"""
102102

103-
def __init__(self, *, name: str | None = None) -> None:
103+
def __init__(self, *, unique_id: str | None = None) -> None:
104104
"""Initialize this BackgroundService.
105105
106106
Args:
107-
name: The name of this background service. If `None`, `str(id(self))` will
108-
be used. This is used mostly for debugging purposes.
107+
unique_id: The string to uniquely identify this background service instance.
108+
If `None`, a string based on `hex(id(self))` will be used. This is
109+
used in `__repr__` and `__str__` methods, mainly for debugging
110+
purposes, to identify a particular instance of a background service.
109111
"""
110-
self._name: str = str(id(self)) if name is None else name
112+
# [2:] is used to remove the '0x' prefix from the hex representation of the id,
113+
# as it doesn't add any uniqueness to the string.
114+
self._unique_id: str = hex(id(self))[2:] if unique_id is None else unique_id
111115
self._tasks: set[asyncio.Task[Any]] = set()
112116

113117
@abc.abstractmethod
114118
def start(self) -> None:
115119
"""Start this background service."""
116120

117121
@property
118-
def name(self) -> str:
119-
"""The name of this background service.
122+
def unique_id(self) -> str:
123+
"""The unique ID of this background service.
120124
121125
Returns:
122-
The name of this background service.
126+
The unique ID of this background service.
123127
"""
124-
return self._name
128+
return self._unique_id
125129

126130
@property
127131
def tasks(self) -> collections.abc.Set[asyncio.Task[Any]]:
@@ -271,12 +275,12 @@ def __repr__(self) -> str:
271275
Returns:
272276
A string representation of this instance.
273277
"""
274-
return f"{type(self).__name__}(name={self._name!r}, tasks={self._tasks!r})"
278+
return f"{type(self).__name__}(unique_id={self._unique_id!r}, tasks={self._tasks!r})"
275279

276280
def __str__(self) -> str:
277281
"""Return a string representation of this instance.
278282
279283
Returns:
280284
A string representation of this instance.
281285
"""
282-
return f"{type(self).__name__}[{self._name}]"
286+
return f"{type(self).__name__}[{self._unique_id}]"

tests/test_asyncio.py

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@ class FakeService(BackgroundService):
2525
def __init__(
2626
self,
2727
*,
28-
name: str | None = None,
28+
unique_id: str | None = None,
2929
sleep: float | None = None,
3030
exc: BaseException | None = None,
3131
) -> None:
3232
"""Initialize a new FakeService."""
33-
super().__init__(name=name)
33+
super().__init__(unique_id=unique_id)
3434
self._sleep = sleep
3535
self._exc = exc
3636

@@ -49,25 +49,28 @@ async def nop() -> None:
4949
async def test_construction_defaults() -> None:
5050
"""Test the construction of a background service with default arguments."""
5151
fake_service = FakeService()
52-
assert fake_service.name == str(id(fake_service))
52+
assert fake_service.unique_id == hex(id(fake_service))[2:]
5353
assert fake_service.tasks == set()
5454
assert fake_service.is_running is False
55-
assert str(fake_service) == f"FakeService[{fake_service.name}]"
56-
assert repr(fake_service) == f"FakeService(name={fake_service.name!r}, tasks=set())"
55+
assert str(fake_service) == f"FakeService[{fake_service.unique_id}]"
56+
assert (
57+
repr(fake_service)
58+
== f"FakeService(unique_id={fake_service.unique_id!r}, tasks=set())"
59+
)
5760

5861

5962
async def test_construction_custom() -> None:
60-
"""Test the construction of a background service with a custom name."""
61-
fake_service = FakeService(name="test")
62-
assert fake_service.name == "test"
63+
"""Test the construction of a background service with a custom unique ID."""
64+
fake_service = FakeService(unique_id="test")
65+
assert fake_service.unique_id == "test"
6366
assert fake_service.tasks == set()
6467
assert fake_service.is_running is False
6568

6669

6770
async def test_start_await() -> None:
6871
"""Test a background service starts and can be awaited."""
69-
fake_service = FakeService(name="test")
70-
assert fake_service.name == "test"
72+
fake_service = FakeService(unique_id="test")
73+
assert fake_service.unique_id == "test"
7174
assert fake_service.is_running is False
7275

7376
# Is a no-op if the service is not running
@@ -86,8 +89,8 @@ async def test_start_await() -> None:
8689

8790
async def test_start_stop() -> None:
8891
"""Test a background service starts and stops correctly."""
89-
fake_service = FakeService(name="test", sleep=2.0)
90-
assert fake_service.name == "test"
92+
fake_service = FakeService(unique_id="test", sleep=2.0)
93+
assert fake_service.unique_id == "test"
9194
assert fake_service.is_running is False
9295

9396
# Is a no-op if the service is not running
@@ -113,8 +116,8 @@ async def test_start_and_crash(
113116
) -> None:
114117
"""Test a background service reports when crashing."""
115118
exc = RuntimeError("error")
116-
fake_service = FakeService(name="test", exc=exc)
117-
assert fake_service.name == "test"
119+
fake_service = FakeService(unique_id="test", exc=exc)
120+
assert fake_service.unique_id == "test"
118121
assert fake_service.is_running is False
119122

120123
fake_service.start()
@@ -141,7 +144,7 @@ async def test_start_and_crash(
141144

142145
async def test_async_context_manager() -> None:
143146
"""Test a background service works as an async context manager."""
144-
async with FakeService(name="test", sleep=1.0) as fake_service:
147+
async with FakeService(unique_id="test", sleep=1.0) as fake_service:
145148
assert fake_service.is_running is True
146149
# Is a no-op if the service is running
147150
fake_service.start()

0 commit comments

Comments
 (0)