Add _all param to generate_runs

[skycastlelily]

Pull Request Checklist

• [x] implement the feature
• [ ] write the documentation
• [ ] extend the test coverage
• [ ] update the specification
• [ ] adjust plugin docstring
• [ ] modify the json schema
• [ ] mention the version
• [ ] include a release note

[skycastlelily]

Fix this issue:https://github.com/teemtee/tmt/issues/2874^^

[skycastlelily]

I didn't realize run.yaml will be used for cleaning guests😅, and I am working on having try create run.yaml approach,this mr will fix the problem of removing runs from tmt try, as run.yaml will not be used for clean runs, so a dummy one works:)

On Wed, Apr 24, 2024 at 3:39 PM Miloš Prchlík @.***> wrote:

@.**** commented on this pull request.

In tmt/utils.py https://github.com/teemtee/tmt/pull/2882#discussion_r1577427084:

@@ -4618,6 +4618,9 @@ def generate_runs( # in abs_path to be generated. invalid_id = id_ and str(abs_child_path) != id_ invalid_run = not abs_child_path.joinpath('run.yaml').exists()

•    if invalid_run and abs_child_path.name.startswith('run-'):
  

•        os.system(f"touch {abs_child_path.joinpath('run.yaml')}")
  

•        invalid_run = False
  

I'm not sure I follow what's happening and why this helps. The commit message does very little in terms of explaining what's changing and especially why it needs to change. If a workdir lacks run.yaml, what help would it be to create a dummy, empty one? This is unclear from the PR. Can you shed more light on why a change like this solves the problem?

— Reply to this email directly, view it on GitHub https://github.com/teemtee/tmt/pull/2882#pullrequestreview-2019105028, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKFR23FAPQ74NVL4ETSI2DDY65ORZAVCNFSM6AAAAABGVB7GKKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMJZGEYDKMBSHA . You are receiving this because you authored the thread.Message ID: @.***>

[skycastlelily]

I have created a new pull request to fix the try specific issue: https://github.com/teemtee/tmt/pull/2889^^ For general clean runs that doesn't work without the run.yaml issue,we may still need this merge request.

Is the change getmtime -> getatime intentional?

Yep,as we create the run.yaml for the runs which doesn't have that file, and the getmtime will be the time of run.yaml is created in that case. We need check the getatime,which is the time of the dir created^^

Also, if run.yaml will not be used for clean runs, does it need to exist? If we add a flag to generate_runs() to include also workdirs without run.yaml

Ah,right,we could add a flag😄and then we doesn't need to create the dummy run.yaml any more^^

On Thu, Apr 25, 2024 at 4:58 PM Miloš Prchlík @.***> wrote:

@.**** commented on this pull request.

In tmt/utils.py https://github.com/teemtee/tmt/pull/2882#discussion_r1579128732:

@@ -4618,6 +4618,9 @@ def generate_runs( # in abs_path to be generated. invalid_id = id_ and str(abs_child_path) != id_ invalid_run = not abs_child_path.joinpath('run.yaml').exists()

•    if invalid_run and abs_child_path.name.startswith('run-'):
  

•        os.system(f"touch {abs_child_path.joinpath('run.yaml')}")
  

•        invalid_run = False
  

I didn't realize run.yaml will be used for cleaning guests😅, and I am working on having try create run.yaml approach,this mr will fix the problem of removing runs from tmt try, as run.yaml will not be used for clean runs, so a dummy one works:)

Isn't it changing the way of discovering workdir for every user of this function? Let's say that for cleaning guests, run.yaml is not important and may not exist, but what about others? This function gets called from Status.show(), and it starts returning workdirs that were considered invalid in the past, which is not related to "cleaning guests" use case.

Also, if run.yaml will not be used for clean runs, does it need to exist? If we add a flag to generate_runs() to include also workdirs without run.yaml, does it have to be created? That might fool other parts of tmt into thinking this workdir is suddenly having apparently valid run.yaml, and therefore it's a valid workdir and should be inspected...

— Reply to this email directly, view it on GitHub https://github.com/teemtee/tmt/pull/2882#discussion_r1579128732, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKFR23FH6LBMT6WFKXHGWRDY7DATJAVCNFSM6AAAAABGVB7GKKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMRRHE2DANJZG4 . You are receiving this because you authored the thread.Message ID: @.***>

[happz]

Is the change getmtime -> getatime intentional? Yep,as we create the run.yaml for the runs which doesn't have that file, and the getmtime will be the time of run.yaml is created in that case. We need check the getatime,which is the time of the dir created^^

"atime" is the last time the directory was "read", i.e. someone tried listing its content. It's not related to when the directory was created, that would be "ctime". But directories also have "mtime", the time when the directory was "modified", i.e. someone changed its content.

[skycastlelily]

But I checked, the output of os.path.getctime is the same as os.path.getmtime, which is the time that file is created:)

On Thu, Apr 25, 2024 at 5:43 PM Miloš Prchlík @.***> wrote:

Is the change getmtime -> getatime intentional? Yep,as we create the run.yaml for the runs which doesn't have that file, and the getmtime will be the time of run.yaml is created in that case. We need check the getatime,which is the time of the dir created^^

"atime" is the last time the directory was "read", i.e. someone tried listing its content. It's not related to when the directory was created, that would be "ctime". But directories also have "mtime", the time when the directory was "modified", i.e. someone changed its content.

— Reply to this email directly, view it on GitHub https://github.com/teemtee/tmt/pull/2882#issuecomment-2076783211, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKFR23A7GII3BEKAYE5O5XTY7DF2BAVCNFSM6AAAAABGVB7GKKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZWG44DGMRRGE . You are receiving this because you authored the thread.Message ID: @.***>

[happz]

But I checked, the output of os.path.getctime is the same as os.path.getmtime, which is the time that file is created:)

No, "mtime" is not the time a file (or directory) was created, it's the time it was modified. It might have been same, that may happen, see below, but it's not the same property and they will be different sooner or later. If you treat atime as "the time file/directory was created", it will bite you.

happz@multivac ~ $ mkdir /tmp/foo
happz@multivac ~ $ stat /tmp/foo
...
# just created, all times are the same
Access: 2024-04-25 12:09:44.957668243 +0200
Modify: 2024-04-25 12:09:44.957668243 +0200
Change: 2024-04-25 12:09:44.957668243 +0200
 Birth: 2024-04-25 12:09:44.957668243 +0200

happz@multivac ~ $ touch /tmp/foo/bar.txt
happz@multivac ~ $ stat /tmp/foo
...
# created a file - "modified" the directory -> access/birth did not change, modify changed
Access: 2024-04-25 12:09:44.957668243 +0200
Modify: 2024-04-25 12:10:01.693803605 +0200
Change: 2024-04-25 12:10:01.693803605 +0200
 Birth: 2024-04-25 12:09:44.957668243 +0200

happz@multivac ~ $ ls /tmp/foo
bar.txt
happz@multivac ~ $ stat /tmp/foo
...
# listed the directory - "read" the directory -> access changed, but not modify or birth
Access: 2024-04-25 12:10:22.929975365 +0200
Modify: 2024-04-25 12:10:01.693803605 +0200
Change: 2024-04-25 12:10:01.693803605 +0200
 Birth: 2024-04-25 12:09:44.957668243 +0200

happz@multivac ~ $ stat /tmp/foo/bar.txt
...
# the file was created and left untouched, all times are the same
Access: 2024-04-25 12:10:01.693803605 +0200
Modify: 2024-04-25 12:10:01.693803605 +0200
Change: 2024-04-25 12:10:01.693803605 +0200
 Birth: 2024-04-25 12:10:01.693803605 +0200

happz@multivac ~ $ echo "baz" > /tmp/foo/bar.txt
happz@multivac ~ $ stat /tmp/foo/bar.txt
...
# writing to the file - modify/change are different, not the access/creation
Access: 2024-04-25 12:10:01.693803605 +0200
Modify: 2024-04-25 12:11:33.312565817 +0200
Change: 2024-04-25 12:11:33.312565817 +0200
 Birth: 2024-04-25 12:10:01.693803605 +0200

happz@multivac ~ $ cat /tmp/foo/bar.txt
baz
happz@multivac ~ $ stat /tmp/foo/bar.txt
...
# and after reading the file, access is now different that modify and birth,
# and all three properties - ctime, atime and mtime hold different values.
Access: 2024-04-25 12:14:32.721916944 +0200
Modify: 2024-04-25 12:11:33.312565817 +0200
Change: 2024-04-25 12:11:33.312565817 +0200
 Birth: 2024-04-25 12:10:01.693803605 +0200
happz@multivac ~ $ 

[skycastlelily]

ah,right, thanks, professional^^, for your mentor. Luckily, we don't need to create a dummy run.yaml file any more:)

On Thu, Apr 25, 2024 at 6:19 PM Miloš Prchlík @.***> wrote:

But I checked, the output of os.path.getctime is the same as os.path.getmtime, which is the time that file is created:)

No, "mtime" is not the time a file (or directory) was created, it's the time it was modified. It might have been same, that may happen, see below, but it's not the same property and they will be different sooner or later. If you treat atime as "the time file/directory was created", it will bite you.

@.*** ~ $ mkdir /tmp/foo @.*** ~ $ stat /tmp/foo ...

just created, all times are the same

Access: 2024-04-25 12:09:44.957668243 +0200 Modify: 2024-04-25 12:09:44.957668243 +0200 Change: 2024-04-25 12:09:44.957668243 +0200 Birth: 2024-04-25 12:09:44.957668243 +0200

@.*** ~ $ touch /tmp/foo/bar.txt @.*** ~ $ stat /tmp/foo ...

created a file - "modified" the directory -> access/birth did not change, modify changed

Access: 2024-04-25 12:09:44.957668243 +0200 Modify: 2024-04-25 12:10:01.693803605 +0200 Change: 2024-04-25 12:10:01.693803605 +0200 Birth: 2024-04-25 12:09:44.957668243 +0200

@.*** ~ $ ls /tmp/foo bar.txt @.*** ~ $ stat /tmp/foo ...

listed the directory - "read" the directory -> access changed, but not modify or birth

Access: 2024-04-25 12:10:22.929975365 +0200 Modify: 2024-04-25 12:10:01.693803605 +0200 Change: 2024-04-25 12:10:01.693803605 +0200 Birth: 2024-04-25 12:09:44.957668243 +0200

@.*** ~ $ stat /tmp/foo/bar.txt ...

the file was created and left untouched, all times are the same

Access: 2024-04-25 12:10:01.693803605 +0200 Modify: 2024-04-25 12:10:01.693803605 +0200 Change: 2024-04-25 12:10:01.693803605 +0200 Birth: 2024-04-25 12:10:01.693803605 +0200

@.*** ~ $ echo "baz" > /tmp/foo/bar.txt @.*** ~ $ stat /tmp/foo/bar.txt ...

writing to the file - modify/change are different, not the access/creation

Access: 2024-04-25 12:10:01.693803605 +0200 Modify: 2024-04-25 12:11:33.312565817 +0200 Change: 2024-04-25 12:11:33.312565817 +0200 Birth: 2024-04-25 12:10:01.693803605 +0200

@.*** ~ $ cat /tmp/foo/bar.txt baz @.*** ~ $ stat /tmp/foo/bar.txt ...

and after reading the file, access is now different that modify and birth,

and all three properties - ctime, atime and mtime hold different values.

Access: 2024-04-25 12:14:32.721916944 +0200 Modify: 2024-04-25 12:11:33.312565817 +0200 Change: 2024-04-25 12:11:33.312565817 +0200 Birth: 2024-04-25 12:10:01.693803605 +0200 @.*** ~ $

— Reply to this email directly, view it on GitHub https://github.com/teemtee/tmt/pull/2882#issuecomment-2076849139, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKFR23D5ZGPRVLI6EFYI5VTY7DKDPAVCNFSM6AAAAABGVB7GKKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZWHA2DSMJTHE . You are receiving this because you authored the thread.Message ID: @.***>

[skycastlelily]

Updated^^

[skycastlelily]

I changed my idea,as I found this is really a tiny and clean one, and it would be great if we could have this reviewed during 1.41^^

[thrix]

I am sorry moving this again, please bring this to hacking, seems this needs more discussion.

[skycastlelily]

If a user wants to use tmt clean runs together with --keep option, then all_workdirs will contain also workdirs that don't have run.yaml file and it could throw an error during sorting here. I would suggest sorting by modification time of the workdir itself, if the directory doesn't contain run.yaml file.

ah, good catch, thanks,updated:) https://github.com/teemtee/tmt/pull/2882/commits/41a4c922d44a8a84128145054d77a102f338f003