From 63879df297b3a7d0bc0940f89a06d6b3c72d5015 Mon Sep 17 00:00:00 2001 From: Asher Pemberton Date: Mon, 20 Jul 2026 09:23:11 +0000 Subject: [PATCH] feat: rename LG_TOKEN to LG_RESERVATION Rename reservation tokens to reservation IDs throughout the client, coordinator, documentation, and tests. Export new reservations through LG_RESERVATION, while continuing to accept the legacy LG_TOKEN variable with a deprecation warning for backwards compatibility. Signed-off-by: Asher Pemberton Reviewed-by: Asher Pemberton # gatekeeper Co-authored-by: Luke Beardsmore --- doc/man/client.rst | 4 +-- doc/usage.rst | 29 +++++++++++------ labgrid/remote/client.py | 61 +++++++++++++++++++++-------------- labgrid/remote/common.py | 8 ++--- labgrid/remote/coordinator.py | 44 ++++++++++++------------- man/labgrid-client.1 | 12 ++++--- tests/test_client.py | 25 +++++++------- tests/test_pb2.py | 6 ++-- 8 files changed, 107 insertions(+), 82 deletions(-) diff --git a/doc/man/client.rst b/doc/man/client.rst index 309e36365..f5b25be24 100644 --- a/doc/man/client.rst +++ b/doc/man/client.rst @@ -25,8 +25,8 @@ LG_PLACE ~~~~~~~~ This variable can be used to specify a place without using the ``-p`` option, the ``-p`` option overrides it. -LG_TOKEN -~~~~~~~~ +LG_RESERVATION (previously LG_TOKEN) +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This variable can be used to specify a reservation for the ``wait`` command and for the ``+`` place expansion. diff --git a/doc/usage.rst b/doc/usage.rst index 6f743d0c7..11a272ecf 100644 --- a/doc/usage.rst +++ b/doc/usage.rst @@ -67,7 +67,7 @@ tags). $ labgrid-client reserve board=imx6-foo Reservation 'SP37P5OQRU': owner: rettich/jlu - token: SP37P5OQRU + id : SP37P5OQRU state: waiting filters: main: board=imx6-foo @@ -76,7 +76,7 @@ tags). As soon as any matching place becomes free, the reservation state will change from ``waiting`` to ``allocated``. -Then, you can use the reservation token prefixed by ``+`` to refer to the +Then, you can use the reservation id prefixed by ``+`` to refer to the allocated place for locking and usage. While a place is allocated for a reservation, only the owner of the reservation can lock that place. @@ -86,7 +86,7 @@ can lock that place. $ labgrid-client wait SP37P5OQRU owner: rettich/jlu - token: SP37P5OQRU + id: SP37P5OQRU state: waiting filters: main: board=imx6-foo @@ -94,7 +94,7 @@ can lock that place. timeout: 2019-08-06 12:58:14.900621 … owner: rettich/jlu - token: SP37P5OQRU + id: SP37P5OQRU state: allocated filters: main: board=imx6-foo @@ -107,7 +107,7 @@ can lock that place. $ labgrid-client reservations Reservation 'SP37P5OQRU': owner: rettich/jlu - token: SP37P5OQRU + id: SP37P5OQRU state: acquired filters: main: board=imx6-foo @@ -117,20 +117,31 @@ can lock that place. timeout: 2019-08-06 12:59:11.840780 $ labgrid-client -p +SP37P5OQRU console + +.. note:: + + Reservation tokens are now called reservation IDs. + As part of this terminology change, ``LG_TOKEN`` has been renamed to + ``LG_RESERVATION``, and ``labgrid-client reserve --shell`` now exports + ``LG_RESERVATION``. + Command output now uses ``id`` instead of ``token``. + The legacy ``LG_TOKEN`` variable remains accepted as input for backwards + compatibility, but scripts should migrate to ``LG_RESERVATION``. + When using reservation in a CI job or to save some typing, the ``labgrid-client reserve`` command supports a ``--shell`` command to print code for evaluating in the shell. -This sets the ``LG_TOKEN`` environment variable, which is then automatically +This sets the ``LG_RESERVATION`` environment variable, which is then automatically used by ``wait`` and expanded via ``-p +``. .. code-block:: bash $ eval `labgrid-client reserve --shell board=imx6-foo` - $ echo $LG_TOKEN + $ echo $LG_RESERVATION ZDMZJZNLBF $ labgrid-client wait owner: rettich/jlu - token: ZDMZJZNLBF + id: ZDMZJZNLBF state: waiting filters: main: board=imx6-foo @@ -138,7 +149,7 @@ used by ``wait`` and expanded via ``-p +``. timeout: 2019-08-06 13:06:44.629736 … owner: rettich/jlu - token: ZDMZJZNLBF + id: ZDMZJZNLBF state: allocated filters: main: board=imx6-foo diff --git a/labgrid/remote/client.py b/labgrid/remote/client.py index 069240b9e..9d07bf697 100755 --- a/labgrid/remote/client.py +++ b/labgrid/remote/client.py @@ -86,6 +86,17 @@ def __str__(self): return f"{self.message}:\n{errors_combined}" +def _get_reservation_id_from_env(): + reservation_id = os.environ.get("LG_RESERVATION") + if reservation_id: + return reservation_id + + reservation_id = os.environ.get("LG_TOKEN") + if reservation_id: + print("warning: LG_TOKEN is deprecated; use LG_RESERVATION instead", file=sys.stderr) + return reservation_id + + @attr.s(eq=False) class ClientSession: """The ClientSession encapsulates all the actions a Client can invoke on @@ -438,19 +449,19 @@ def _match_places(self, pattern): """ result = set() - # reservation token lookup - token = None + # reservation id lookup + reservation_id = None if pattern.startswith("+"): - token = pattern[1:] - if not token: - token = os.environ.get("LG_TOKEN", None) - if not token: + reservation_id = pattern[1:] + if not reservation_id: + reservation_id = _get_reservation_id_from_env() + if not reservation_id: return [] for name, place in self.places.items(): - if place.reservation == token: + if place.reservation == reservation_id: result.add(name) if not result: - raise UserError(f"reservation token {token} matches nothing") + raise UserError(f"reservation id {reservation_id} matches nothing") return list(result) # name and alias lookup @@ -1572,28 +1583,28 @@ async def create_reservation(self): res = Reservation.from_pb2(response.reservation) if self.args.shell: - print(f"export LG_TOKEN={res.token}") + print(f"export LG_RESERVATION={res.id}") else: - print(f"Reservation '{res.token}':") + print(f"Reservation '{res.id}':") res.show(level=1) if self.args.wait: if not self.args.shell: print("Waiting for allocation...") - await self._wait_reservation(res.token, verbose=False) + await self._wait_reservation(res.id, verbose=False) async def cancel_reservation(self): - token: str = self.args.token + reservation_id: str = getattr(self.args, "reservation-id") - request = labgrid_coordinator_pb2.CancelReservationRequest(token=token) + request = labgrid_coordinator_pb2.CancelReservationRequest(token=reservation_id) try: await self.stub.CancelReservation(request) except grpc.aio.AioRpcError as e: raise ServerError(e.details()) - async def _wait_reservation(self, token: str, verbose=True): + async def _wait_reservation(self, reservation_id: str, verbose=True): while True: - request = labgrid_coordinator_pb2.PollReservationRequest(token=token) + request = labgrid_coordinator_pb2.PollReservationRequest(token=reservation_id) try: response: labgrid_coordinator_pb2.PollReservationResponse = await self.stub.PollReservation(request) @@ -1609,8 +1620,8 @@ async def _wait_reservation(self, token: str, verbose=True): break async def wait_reservation(self): - token = self.args.token - await self._wait_reservation(token) + reservation_id = getattr(self.args, "reservation-id") + await self._wait_reservation(reservation_id) async def print_reservations(self): request = labgrid_coordinator_pb2.GetReservationsRequest() @@ -1622,7 +1633,7 @@ async def print_reservations(self): raise ServerError(e.details()) for res in sorted(reservations, key=lambda x: (-x.prio, x.created)): - print(f"Reservation '{res.token}':") + print(f"Reservation '{res.id}':") res.show(level=1) async def export(self, place, target): @@ -2210,11 +2221,11 @@ def get_parser(auto_doc_mode=False) -> "argparse.ArgumentParser | AutoProgramArg subparser.set_defaults(func=ClientSession.create_reservation) subparser = subparsers.add_parser("cancel-reservation", help="cancel a reservation") - subparser.add_argument("token", type=str, nargs="?") + subparser.add_argument("reservation-id", type=str, nargs="?", help="the reservation id (previously called token)") subparser.set_defaults(func=ClientSession.cancel_reservation) subparser = subparsers.add_parser("wait", help="wait for a reservation to be allocated") - subparser.add_argument("token", type=str, nargs="?") + subparser.add_argument("reservation-id", type=str, nargs="?", help="the reservation id (previously called token)") subparser.set_defaults(func=ClientSession.wait_reservation) subparser = subparsers.add_parser("reservations", help="list current reservations") @@ -2257,7 +2268,6 @@ def main(): state = os.environ.get("STATE", None) state = os.environ.get("LG_STATE", state) initial_state = os.environ.get("LG_INITIAL_STATE", None) - token = os.environ.get("LG_TOKEN", None) parser = get_parser() @@ -2281,11 +2291,12 @@ def main(): if args.initial_state is None: args.initial_state = initial_state - if args.command in ["cancel-reservation", "wait"] and args.token is None: - if token: - args.token = token + if args.command in ["cancel-reservation", "wait"] and getattr(args, "reservation-id") is None: + reservation_id = _get_reservation_id_from_env() + if reservation_id: + setattr(args, "reservation-id", reservation_id) else: - print("Please provide a token", file=sys.stderr) + print("Please provide a reservation id (previously called token)", file=sys.stderr) exit(1) if args.verbose: diff --git a/labgrid/remote/common.py b/labgrid/remote/common.py index 14c8a2d74..5ac48c77c 100644 --- a/labgrid/remote/common.py +++ b/labgrid/remote/common.py @@ -391,7 +391,7 @@ class ReservationState(enum.Enum): @attr.s(eq=False) class Reservation: owner = attr.ib(validator=attr.validators.instance_of(str)) - token = attr.ib( + id = attr.ib( default=attr.Factory(lambda: "".join(random.choice(string.ascii_uppercase + string.digits) for i in range(10))) ) state = attr.ib( @@ -428,7 +428,7 @@ def expired(self): def show(self, level=0): indent = " " * level print(indent + f"owner: {self.owner}") - print(indent + f"token: {self.token}") + print(indent + f"id: {self.id}") print(indent + f"state: {self.state.name}") if self.prio: print(indent + f"prio: {self.prio}") @@ -445,7 +445,7 @@ def show(self, level=0): def as_pb2(self): res = labgrid_coordinator_pb2.Reservation() res.owner = self.owner - res.token = self.token + res.token = self.id res.state = self.state.value res.prio = self.prio for name, fltr in self.filters.items(): @@ -471,7 +471,7 @@ def from_pb2(cls, pb2: labgrid_coordinator_pb2.Reservation): allocations[fltr_name] = [place_name] return cls( owner=pb2.owner, - token=pb2.token, + id=pb2.token, state=ReservationState(pb2.state), prio=pb2.prio, filters=filters, diff --git a/labgrid/remote/coordinator.py b/labgrid/remote/coordinator.py index 3423f3cef..5cd00a29e 100644 --- a/labgrid/remote/coordinator.py +++ b/labgrid/remote/coordinator.py @@ -968,10 +968,10 @@ def schedule_reservations(self): res.state = ReservationState.expired res.allocations.clear() res.refresh() - print(f"reservation ({res.owner}/{res.token}) is now {res.state.name}") + print(f"reservation ({res.owner}/{res.id}) is now {res.state.name}") else: - del self.reservations[res.token] - print(f"removed {res.state.name} reservation ({res.owner}/{res.token})") + del self.reservations[res.id] + print(f"removed {res.state.name} reservation ({res.owner}/{res.id})") # check which places are already allocated and handle state transitions allocated_places = set() @@ -985,7 +985,7 @@ def schedule_reservations(self): res.state = ReservationState.invalid res.allocations.clear() res.refresh(300) - print(f"reservation ({res.owner}/{res.token}) is now {res.state.name}") + print(f"reservation ({res.owner}/{res.id}) is now {res.state.name}") break if place.acquired is not None: acquired_places.add(name) @@ -995,12 +995,12 @@ def schedule_reservations(self): # an allocated place was acquired res.state = ReservationState.acquired res.refresh() - print(f"reservation ({res.owner}/{res.token}) is now {res.state.name}") + print(f"reservation ({res.owner}/{res.id}) is now {res.state.name}") if not acquired_places and res.state is ReservationState.acquired: # all allocated places were released res.state = ReservationState.allocated res.refresh() - print(f"reservation ({res.owner}/{res.token}) is now {res.state.name}") + print(f"reservation ({res.owner}/{res.id}) is now {res.state.name}") # check which places are available for allocation available_places = set() @@ -1027,16 +1027,16 @@ def schedule_reservations(self): place_tagsets.append(TagSet(name, tags)) filter_tagsets = [] for res in pending_reservations: - filter_tagsets.append(TagSet(res.token, set(res.filters["main"].items()))) + filter_tagsets.append(TagSet(res.id, set(res.filters["main"].items()))) allocation = schedule(place_tagsets, filter_tagsets) # apply allocations - for res_token, place_name in allocation.items(): - res = self.reservations[res_token] + for res_id, place_name in allocation.items(): + res = self.reservations[res_id] res.allocations = {"main": [place_name]} res.state = ReservationState.allocated res.refresh() - print(f"reservation ({res.owner}/{res.token}) is now {res.state.name}") + print(f"reservation ({res.owner}/{res.id}) is now {res.state.name}") # update reservation property of each place and notify old_map = {} @@ -1051,10 +1051,10 @@ def schedule_reservations(self): for group in res.allocations.values(): for name in group: assert name not in new_map, "conflicting allocation" - new_map[name] = res.token + new_map[name] = res.id place = self.places.get(name) assert place is not None, "invalid allocation" - place.reservation = res.token + place.reservation = res.id for name in old_map.keys() | new_map.keys(): if old_map.get(name) != new_map.get(name): self._publish_place(self.places[name]) @@ -1079,28 +1079,28 @@ async def CreateReservation(self, request: labgrid_coordinator_pb2.CreateReserva owner = self.clients[peer].name res = Reservation(owner=owner, prio=request.prio, filters=fltrs) - self.reservations[res.token] = res + self.reservations[res.id] = res self.schedule_reservations() return labgrid_coordinator_pb2.CreateReservationResponse(reservation=res.as_pb2()) @locked async def CancelReservation(self, request: labgrid_coordinator_pb2.CancelReservationRequest, context): - token = request.token - if not isinstance(token, str) or not token: - await context.abort(grpc.StatusCode.INVALID_ARGUMENT, f"Invalid token {token}") - if token not in self.reservations: - await context.abort(grpc.StatusCode.FAILED_PRECONDITION, f"Reservation {token} does not exist") - del self.reservations[token] + reservation_id = request.token + if not isinstance(reservation_id, str) or not reservation_id: + await context.abort(grpc.StatusCode.INVALID_ARGUMENT, f"Invalid id {reservation_id}") + if reservation_id not in self.reservations: + await context.abort(grpc.StatusCode.FAILED_PRECONDITION, f"Reservation {reservation_id} does not exist") + del self.reservations[reservation_id] self.schedule_reservations() return labgrid_coordinator_pb2.CancelReservationResponse() @locked async def PollReservation(self, request: labgrid_coordinator_pb2.PollReservationRequest, context): - token = request.token + reservation_id = request.token try: - res = self.reservations[token] + res = self.reservations[reservation_id] except KeyError: - await context.abort(grpc.StatusCode.FAILED_PRECONDITION, f"Reservation {token} does not exist") + await context.abort(grpc.StatusCode.FAILED_PRECONDITION, f"Reservation {reservation_id} does not exist") res.refresh() return labgrid_coordinator_pb2.PollReservationResponse(reservation=res.as_pb2()) diff --git a/man/labgrid-client.1 b/man/labgrid-client.1 index 45533af05..5a1dae153 100644 --- a/man/labgrid-client.1 +++ b/man/labgrid-client.1 @@ -218,13 +218,14 @@ cancel a reservation .INDENT 3.5 .sp .EX -usage: labgrid\-client cancel\-reservation [token] +usage: labgrid\-client cancel\-reservation [reservation\-id] .EE .UNINDENT .UNINDENT .INDENT 0.0 .TP -.B token +.B reservation\-id +the reservation id (previously called token) .UNINDENT .SS labgrid\-client console|con .sp @@ -997,13 +998,14 @@ wait for a reservation to be allocated .INDENT 3.5 .sp .EX -usage: labgrid\-client wait [token] +usage: labgrid\-client wait [reservation\-id] .EE .UNINDENT .UNINDENT .INDENT 0.0 .TP -.B token +.B reservation\-id +the reservation id (previously called token) .UNINDENT .SS labgrid\-client who .sp @@ -1125,7 +1127,7 @@ Various labgrid\-client commands use the following environment variable: .SS LG_PLACE .sp This variable can be used to specify a place without using the \fB\-p\fP option, the \fB\-p\fP option overrides it. -.SS LG_TOKEN +.SS LG_RESERVATION (previously LG_TOKEN) .sp This variable can be used to specify a reservation for the \fBwait\fP command and for the \fB+\fP place expansion. diff --git a/tests/test_client.py b/tests/test_client.py index 8d2cc1c9f..ee1f5d2bf 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -361,19 +361,20 @@ def test_reservation(place_acquire, tmpdir): spawn.expect(pexpect.EOF) spawn.close() assert spawn.exitstatus == 0, spawn.before.strip() - m = re.search(rb"^export LG_TOKEN=(\S+)$", spawn.before.replace(b'\r\n', b'\n'), re.MULTILINE) + m = re.search(rb"^export LG_RESERVATION=(\S+)$", spawn.before.replace(b'\r\n', b'\n'), re.MULTILINE) assert m is not None, spawn.before.strip() - token = m.group(1) + reservation_id = m.group(1) env = os.environ.copy() - env['LG_TOKEN'] = token.decode('ASCII') + # Use LG_TOKEN in this test to validate backwards compatibility. Other tests use LG_RESERVATION. + env['LG_TOKEN'] = reservation_id.decode('ASCII') with pexpect.spawn('python -m labgrid.remote.client reservations') as spawn: spawn.expect(pexpect.EOF) spawn.close() assert spawn.exitstatus == 0, spawn.before.strip() assert b'waiting' in spawn.before, spawn.before.strip() - assert token in spawn.before, spawn.before.strip() + assert reservation_id in spawn.before, spawn.before.strip() with pexpect.spawn('python -m labgrid.remote.client -p test release') as spawn: spawn.expect(pexpect.EOF) @@ -385,14 +386,14 @@ def test_reservation(place_acquire, tmpdir): spawn.close() assert spawn.exitstatus == 0, spawn.before.strip() assert b'allocated' in spawn.before, spawn.before.strip() - assert token in spawn.before, spawn.before.strip() + assert reservation_id in spawn.before, spawn.before.strip() with pexpect.spawn('python -m labgrid.remote.client reservations') as spawn: spawn.expect(pexpect.EOF) spawn.close() assert spawn.exitstatus == 0, spawn.before.strip() assert b'allocated' in spawn.before, spawn.before.strip() - assert token in spawn.before, spawn.before.strip() + assert reservation_id in spawn.before, spawn.before.strip() with pexpect.spawn('python -m labgrid.remote.client -p + acquire', env=env) as spawn: spawn.expect(pexpect.EOF) @@ -402,7 +403,7 @@ def test_reservation(place_acquire, tmpdir): with pexpect.spawn('python -m labgrid.remote.client -p + show', env=env) as spawn: spawn.expect(pexpect.EOF) spawn.close() - assert token in spawn.before, spawn.before.strip() + assert reservation_id in spawn.before, spawn.before.strip() assert spawn.exitstatus == 0, spawn.before.strip() with pexpect.spawn('python -m labgrid.remote.client -p + release', env=env) as spawn: @@ -415,7 +416,7 @@ def test_reservation(place_acquire, tmpdir): spawn.close() assert spawn.exitstatus == 0, spawn.before.strip() assert b'allocated' in spawn.before, spawn.before.strip() - assert token in spawn.before, spawn.before.strip() + assert reservation_id in spawn.before, spawn.before.strip() with pexpect.spawn('python -m labgrid.remote.client cancel-reservation', env=env) as spawn: spawn.expect(pexpect.EOF) @@ -426,7 +427,7 @@ def test_reservation(place_acquire, tmpdir): spawn.expect(pexpect.EOF) spawn.close() assert spawn.exitstatus == 0, spawn.before.strip() - assert token not in spawn.before, spawn.before.strip() + assert reservation_id not in spawn.before, spawn.before.strip() with pexpect.spawn('python -m labgrid.remote.client -p test acquire') as spawn: spawn.expect(pexpect.EOF) @@ -568,14 +569,14 @@ def test_reservation_custom_config(place, exporter, tmpdir): spawn.expect(pexpect.EOF) spawn.close() assert spawn.exitstatus == 0, spawn.before.strip() - m = re.search(rb"^export LG_TOKEN=(\S+)$", spawn.before.replace(b'\r\n', b'\n'), re.MULTILINE) + m = re.search(rb"^export LG_RESERVATION=(\S+)$", spawn.before.replace(b'\r\n', b'\n'), re.MULTILINE) s = re.search(rb"^Selected role$", spawn.before.replace(b'\r\n', b'\n'), re.MULTILINE) assert m is not None, spawn.before.strip() assert s is None, spawn.before.strip() - token = m.group(1) + reservation_id = m.group(1) env = os.environ.copy() - env['LG_TOKEN'] = token.decode('ASCII') + env['LG_RESERVATION'] = reservation_id.decode('ASCII') with pexpect.spawn(f'python -m labgrid.remote.client -c {p} -p + lock', env=env) as spawn: spawn.expect("acquired place test") diff --git a/tests/test_pb2.py b/tests/test_pb2.py index d1340ab45..3c5da470c 100644 --- a/tests/test_pb2.py +++ b/tests/test_pb2.py @@ -86,7 +86,7 @@ def test_reservation_as_pb2(): ) pb2 = reservation.as_pb2() assert pb2.owner == "test" - assert pb2.token == reservation.token + assert pb2.token == reservation.id assert pb2.state == reservation.state.value assert pb2.filters["main"].filter == {"some": "filter"} assert pb2.created == reservation.created @@ -104,7 +104,7 @@ def test_reservation_as_from_pb2(): ) pb2 = resold.as_pb2() assert pb2.owner == resold.owner - assert pb2.token == resold.token + assert pb2.token == resold.id assert pb2.state == resold.state.value assert pb2.filters["main"].filter == {"some": "filter"} assert pb2.created == resold.created @@ -113,7 +113,7 @@ def test_reservation_as_from_pb2(): resnew = Reservation.from_pb2(pb2) assert resnew.owner == resold.owner - assert resnew.token == resold.token + assert resnew.id == resold.id assert resnew.state == resold.state assert resnew.filters["main"] == resold.filters["main"] assert resnew.created == resold.created