-
Notifications
You must be signed in to change notification settings - Fork 123
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
Accept --id option multiple times for clean and status #2891
Conversation
users may want to check status or clean runs/guests of several runs^^ (dev) lnie@fedora:~/tmt$ tmt status --id run-001 --id run-002 (dev) lnie@fedora:~/tmt$ tmt clean runs --id run-001 --id run-002 |
yeah, if users call "tmt clean" in script,updated^^
…On Tue, Jul 9, 2024 at 3:21 AM Miloš Prchlík ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tmt/utils.py
<#2891 (comment)>:
> """ Generate absolute paths to runs from path """
# Prepare absolute workdir path if --id was used
- if id_:
- run_path = Path(id_)
- if '/' not in id_:
- run_path = path / run_path
- if run_path.is_absolute():
- if run_path.exists():
+ run_path = None
+ for id_name in id_:
+ if id_name:
Can id_name not be a valid value? id_ is tuple[str, ...], i.e. all items
will be strings. Can any of them be empty? If so, would it be better to
raise an error rather than ignore --id ""?
—
Reply to this email directly, view it on GitHub
<#2891 (review)>,
or unsubscribe
<https:/notifications/unsubscribe-auth/AKFR23HQPKZRTE2YBDGQBITZLLRBRAVCNFSM6AAAAABG2BLQTSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNRUGE4TEMRUGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Updated^^
…On Wed, Jul 10, 2024 at 3:03 PM Miloš Prchlík ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tmt/utils.py
<#2891 (comment)>:
> yield run_path
- return
+ else:
+ raise tmt.utils.GeneralError("id can not be empty.")
The message could be a bit more helpful :) The world is full of various
"id"s, let's give the user more context, it wouldn't cost us much. How
about something like "Value of '--id' option cannot be an empty string."?
We want to help the user to find the mistake quickly, and we know the value
comes from the --id option, and "empty string" is more specific than
"empty".
—
Reply to this email directly, view it on GitHub
<#2891 (review)>,
or unsubscribe
<https:/notifications/unsubscribe-auth/AKFR23G66Q2R4G4XAEZN6ULZLTMDBAVCNFSM6AAAAABG2BLQTSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNRYGIYDGOBQG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sure,updated:)
…On Tue, Jul 30, 2024 at 9:18 PM Miroslav Vadkerti ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tmt/cli.py
<#2891 (comment)>:
> @@ -1717,7 +1717,7 @@ def try_command(context: Context, image_and_how: str, **kwargs: Any) -> None:
@pass_context
@workdir_root_options
@option(
- '-i', '--id', metavar="ID",
+ '-i', '--id', metavar="ID", multiple=True,
@skycastlelily <https:/skycastlelily> can we update the help
so it is clear it can be specified multiple times?
—
Reply to this email directly, view it on GitHub
<#2891 (review)>,
or unsubscribe
<https:/notifications/unsubscribe-auth/AKFR23CFBXOIESUIVGSG5OLZO6HBRAVCNFSM6AAAAABG2BLQTSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMBXGY3DQMRZGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Updated:)
…On Wed, Jul 31, 2024 at 10:56 PM Filip Vágner ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tmt/base.py
<#2891 (comment)>:
> @@ -3735,12 +3735,11 @@ def show(self) -> None:
""" Display the current status """
# Prepare absolute workdir path if --id was used
# FIXME: cast() - typeless "dispatcher" method
I believe the "FIXME" comment could be deleted as it refers to the id_ =
cast(str, self.opt('id')) line, which is no longer present.
------------------------------
In tmt/base.py
<#2891 (comment)>:
> @@ -3841,15 +3840,14 @@ def guests(self) -> bool:
self.info('guests', color='blue')
root_path = Path(self.opt('workdir-root'))
# FIXME: cast() - typeless "dispatcher" method
Same as above.
------------------------------
In tmt/base.py
<#2891 (comment)>:
> @@ -3877,15 +3875,14 @@ def runs(self) -> bool:
self.info('runs', color='blue')
root_path = Path(self.opt('workdir-root'))
# FIXME: cast() - typeless "dispatcher" method
Same as above.
—
Reply to this email directly, view it on GitHub
<#2891 (review)>,
or unsubscribe
<https:/notifications/unsubscribe-auth/AKFR23FEMIFOFFONDF3NGKLZPD3JPAVCNFSM6AAAAABG2BLQTSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMJQGQYDKNJUG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be nice to have tests for this, but otherwise looks good
Unrelated failures, merging. |
Pull Request Checklist