feat: rename LG_TOKEN to LG_RESERVATION by asher-pem-arm · Pull Request #1932 · labgrid-project/labgrid

asher-pem-arm · 2026-07-13T13:01:18Z

Description

Checklist

Documentation for the feature
Tests for the feature

The arguments and description in doc/configuration.rst have been updated

Add a section on how to use the feature to doc/usage.rst

Add a section on how to use the feature to doc/development.rst

PR has been tested

Man pages have been regenerated

Motivation

Reservation identifiers are not authentication tokens. Calling them IDs makes their purpose clearer and avoids confusion with credentials.

Summary

Rename the reservation identifier from token to reservation ID.

Replace LG_TOKEN with LG_RESERVATION.
Rename Python Reservation.token to Reservation.id.
Update CLI output, help text, documentation, and tests.
Add id support to reservation filters.
Retain LG_TOKEN input and the token filter as legacy fallbacks.
Keep protobuf token fields unchanged for wire compatibility.

Testing

Automated

The following tests have been updated and are all passing.

pytest tests/test_coordinator.py
pytest tests/test_client.py
pytest tests/test_pb2.py

Manual

Reserve, Lock, Unlock (`LG_RESERVATION_ID`)

$ labgrid-client reserve name=p1 --wait --shell
export LG_RESERVATION_ID=YR0SUORW5C
$ LG_RESERVATION_ID=YR0SUORW5C labgrid-client -p + lock
acquired place p1
$ LG_RESERVATION_ID=YR0SUORW5C labgrid-client -p + unlock
released place p1

Reserve, Lock, Unlock (`LG_TOKEN`)

$ labgrid-client reserve name=p1 --wait --shell
export LG_RESERVATION_ID=T9GRNKWWA7
$ LG_TOKEN=T9GRNKWWA7 labgrid-client -p + lock
acquired place p1
$ LG_TOKEN=T9GRNKWWA7 labgrid-client -p + unlock
released place p1

Reserve, Lease, Extend, Release (`LG_RESERVATION_ID`)

$ labgrid-client reserve name=p1 --wait --shell
export LG_RESERVATION_ID=EHA0E0L9OF
LG_RESERVATION_ID=EHA0E0L9OF labgrid-client -p + lease
leased place p1
$ LG_RESERVATION_ID=EHA0E0L9OF labgrid-client extend
extended lease EHA0E0L9OF until 2026-07-07 13:25:00.422546
$ LG_RESERVATION_ID=EHA0E0L9OF labgrid-client -p + release
released place p1

Reserve, Lease, Extend, Release (`LG_TOKEN`)

$ labgrid-client reserve name=p1 --wait --shell
export LG_RESERVATION_ID=HD22S7BBAN
$ LG_TOKEN=HD22S7BBAN labgrid-client -p + lease
leased place p1
$ LG_TOKEN=HD22S7BBAN labgrid-client extend
extended lease HD22S7BBAN until 2026-07-07 13:26:25.733619
$ LG_TOKEN=HD22S7BBAN labgrid-client -p + release
released place p1

Reserve, Wait (`LG_RESERVATION_ID`)

$ labgrid-client reserve name=p1 --wait --shell
export LG_RESERVATION_ID=AOCT0KXTOG
$ LG_RESERVATION_ID=AOCT0KXTOG labgrid-client wait
owner: <machine>/<user>
...

Reserve, Wait (`LG_TOKEN`)

$ labgrid-client reserve name=p1 --wait --shell
export LG_RESERVATION_ID=FDH6EL58TI
$ LG_TOKEN=FDH6EL58TI labgrid-client wait
owner: <machine>/<user>
...

codecov · 2026-07-13T13:13:38Z

Codecov Report

❌ Patch coverage is 77.19298% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.0%. Comparing base (4690e2b) to head (63879df).
⚠️ Report is 18 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
labgrid/remote/coordinator.py	68.1%	7 Missing ⚠️
labgrid/remote/client.py	81.2%	6 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##           master   #1932   +/-   ##
======================================
  Coverage    61.0%   61.0%           
======================================
  Files         182     182           
  Lines       14881   14890    +9     
======================================
+ Hits         9082    9093   +11     
+ Misses       5799    5797    -2

Flag	Coverage Δ
3.10	`61.0% <77.1%> (-0.1%)`	⬇️
3.11	`61.0% <77.1%> (+<0.1%)`	⬆️
3.12	`61.0% <77.1%> (+<0.1%)`	⬆️
3.13	`60.9% <77.1%> (-0.1%)`	⬇️
3.14	`60.9% <77.1%> (-0.1%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

jluebbe · 2026-07-13T13:44:10Z

The downside is that users will likely find old examples/code/talks using LG_TOKEN and become confused because this is no longer mentioned anywhere. Are we sure enough that the new name is so much clearer that it justifies the cost?

I see the possible confusion with authentication tokens, tough. I'm not sure the _ID suffix adds much; LG_RESERVATION would more concise and perhaps clear enough (it could still be called .id internally in the code)?

@Emantor @Bastian-Krause

Emantor · 2026-07-13T13:50:24Z

I do agree on removing the _ID suffix. Is there anything else we might want to add after LG_RESERVATION? Otherwise we can just use LG_RESERVATION without the ID suffix.

Emantor · 2026-07-13T13:51:44Z

+            if not reservation_id:
+                reservation_id = os.environ.get("LG_RESERVATION_ID", None)
+            if not reservation_id:
+                reservation_id = os.environ.get("LG_TOKEN", None)  # For backwards compatibility


We should raise a DeprecationWarning to alert users if the old environment variable LG_TOKEN is used.

Note that DeprecatingWarnings are hidden by default in many cases.

We could have a log message, but I'd like to avoid warnings which are not actionable. Perhaps we should only add the warning in a later release when it's more likely that all components support the new variable.

Leaving as is for now then. Once LG_RESERVATION has been available for a full release and there's a planned removal version then can emit a log warning on stderr when LG_TOKEN is used.

We could have a log message, but I'd like to avoid warnings which are not actionable. Perhaps we should only add the warning in a later release when it's more likely that all components support the new variable.

I would not log, we should instead send a message to stderr to tell them to use the new environment variable instead of the old one. This seems very actionable to me and shortens the cycle with which we can remove the old variable.

yeah good idea, updated

@jluebbe are you okay with these changes?

jluebbe · 2026-07-13T14:46:44Z

The documentation should have a note mentioning the rename, so people have a change to understand what's happening. :)

asher-pem-arm · 2026-07-14T08:57:16Z

updated with rename and docs

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 <asher.pemberton@arm.com> Reviewed-by: Asher Pemberton <asher.pemberton@arm.com> # gatekeeper Co-authored-by: Luke Beardsmore <luke.beardsmore2@arm.com>

Emantor · 2026-07-27T14:01:41Z

This looks good to me, I think we should squash the fixes which should also fix the DCO errors.

asher-pem-arm · 2026-07-27T15:12:59Z

@Emantor thanks, updated

Emantor · 2026-08-07T13:08:20Z

+   ``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``.


This section should export why we are forcing people to change to LG_RESERVATION in all workflows/scripts/CLI usage. IMO this is needed because LG_TOKEN implies a security property (which is not present here) since the meaning of TOKEN has changed over the last few years. Additional explanation or links are welcome as well.

Emantor

The commit message explains what is done, but not why it is done (security implications of the token name) please expand on that.

Emantor · 2026-08-07T13:22:29Z


    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)")


Suggested change

subparser.add_argument("reservation-id", type=str, nargs="?", help="the reservation id (previously called token)")

subparser.add_argument("reservation_id", type=str, nargs="?", help="the reservation id (previously called token)", metavar="reservation-id")

and we can replace the various getattr(args, "reservation-id") calls with args.reservation_id.

Emantor · 2026-08-07T13:24:13Z

        res = Reservation.from_pb2(response.reservation)
        if self.args.shell:
-            print(f"export LG_TOKEN={res.token}")
+            print(f"export LG_RESERVATION={res.id}")


We need to export the old LG_TOKEN here as well at least for a release. Also please add a TODO to eventually remove the LG_TOKEN export print after that.

asher-pem-arm force-pushed the rename-lgtoken-to-lgreservationid branch from e81126a to a5d8bff Compare July 13, 2026 13:06

Emantor requested changes Jul 13, 2026

View reviewed changes

asher-pem-arm requested a review from Emantor July 16, 2026 13:02

Emantor approved these changes Jul 24, 2026

View reviewed changes

Emantor requested a review from Bastian-Krause July 24, 2026 07:08

asher-pem-arm force-pushed the rename-lgtoken-to-lgreservationid branch from aed7bf7 to 63879df Compare July 27, 2026 15:11

asher-pem-arm changed the title ~~feat: rename LG_TOKEN to LG_RESERVATION_ID~~ feat: rename LG_TOKEN to LG_RESERVATION Jul 27, 2026

Emantor requested changes Aug 7, 2026

View reviewed changes

Bastian-Krause added the deprecation label Aug 7, 2026

	subparser.add_argument("reservation-id", type=str, nargs="?", help="the reservation id (previously called token)")
	subparser.add_argument("reservation_id", type=str, nargs="?", help="the reservation id (previously called token)", metavar="reservation-id")

Uh oh!

Uh oh!

Conversation

asher-pem-arm commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Summary

Testing

Automated

Manual

Reserve, Lock, Unlock (LG_RESERVATION_ID)

Reserve, Lock, Unlock (LG_TOKEN)

Reserve, Lease, Extend, Release (LG_RESERVATION_ID)

Reserve, Lease, Extend, Release (LG_TOKEN)

Reserve, Wait (LG_RESERVATION_ID)

Reserve, Wait (LG_TOKEN)

Uh oh!

codecov Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jluebbe commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Emantor commented Jul 13, 2026

Uh oh!

Emantor Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jluebbe commented Jul 13, 2026

Uh oh!

asher-pem-arm commented Jul 14, 2026

Uh oh!

Emantor commented Jul 27, 2026

Uh oh!

asher-pem-arm commented Jul 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Emantor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

asher-pem-arm commented Jul 13, 2026 •

edited

Loading

Reserve, Lock, Unlock (`LG_RESERVATION_ID`)

Reserve, Lock, Unlock (`LG_TOKEN`)

Reserve, Lease, Extend, Release (`LG_RESERVATION_ID`)

Reserve, Lease, Extend, Release (`LG_TOKEN`)

Reserve, Wait (`LG_RESERVATION_ID`)

Reserve, Wait (`LG_TOKEN`)

codecov Bot commented Jul 13, 2026 •

edited

Loading

jluebbe commented Jul 13, 2026 •

edited

Loading

Emantor Jul 13, 2026 •

edited

Loading