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

Add list of available places #1581

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

istepic
Copy link

@istepic istepic commented Jan 27, 2025

Description

Clients sometimes want to know which places are available before acquiring the place.

Checklist

  • Original documentation for the feature already covers this PR, documented on command line, as --acquired.
  • Tests for the feature, no, it's a simple change
  • PR has been tested

Testing:

$ labgrid-client places
target-d-8F94FBAC4EC2DAB50539F2499000055A
target-d-8F94FBAC4EC2DAB50539F299900003D5
$ labgrid-client places --acquired
target-d-8F94FBAC4EC2DAB50539F299900003D5
$ labgrid-client places --available
target-d-8F94FBAC4EC2DAB50539F2499000055A
$ labgrid-client places --help
usage: labgrid-client places [-h] [-a] [-l] [--sort-last-changed]

options:
  -h, --help           show this help message and exit
  -a, --acquired
  -l, --available
  --sort-last-changed  sort by last changed date (oldest first)

@Bastian-Krause
Copy link
Member

I think "available" does not really fit the labgrid lingo, maybe "released" or "unlocked" are better candidates.

@istepic istepic closed this Feb 4, 2025
@istepic istepic reopened this Feb 4, 2025
@istepic
Copy link
Author

istepic commented Feb 4, 2025

@Bastian-Krause makes sense. Done.

$ labgrid-client places -l
target-d-8F94FBAC4EC2DAB50539F2499000055A 
$ labgrid-client places -a
target-d-8F94FBAC4EC2DAB50539F299900003D5 
$ labgrid-client places
target-d-8F94FBAC4EC2DAB50539F2499000055A 
target-d-8F94FBAC4EC2DAB50539F299900003D5 
$ labgrid-client places --released
target-d-8F94FBAC4EC2DAB50539F2499000055A 

Ivan Stepic added 2 commits February 11, 2025 08:02
Signed-off-by: Ivan Stepic <[email protected]>
Add list of released places

Signed-off-by: Ivan Stepic <[email protected]>
@enkiusz
Copy link
Contributor

enkiusz commented Feb 11, 2025

For your consideration, an alternative would be to use:

subparser.add_argument("-a", "--acquired", action=argparse.BooleanOptionalAction, default=None)

and then have --acquired and --no-acquired options used in the loop like so:

        for name, place in places:
            if self.args.acquired is True and place.acquired is None:
                continue
            elif self.args.acquired is False and place.acquired:
                continue
            if self.args.verbose:
                print(f"Place '{name}':")
                place.show(level=1)

This approach does not entail adding a new switch which is like the opposite of --acquired

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants