feat: implement timestamp callback plugin to show simple timestamp for each header #8308

kurokobo · 2024-05-05T13:42:45Z

SUMMARY

This PR introduce new callback plugin to show simple timestamp for each header line.

This plugin reduces the number of *'s in the header by the number of characters in the timestamp, and display the timestamp on the far right.

On the output screen of an AWX job, a timestamp appears in each header by default. This PR implements this timestamp idea as a callback plugin.

There is a plugin that can be used for a similar purpose, ansible.posix.profile_tasks, but it adds a new line for displaying the timestamps, which complicates the output.
The plugin in this PR is as simple as possible and provides only minimal information, so it is not at all obtrusive to keep it enabled at all times.

Available options:

timezone: Users can specify timezone for the timestamp. This is helpful for containerized ansible environment. Ignored on Python < 3.9
format_string: The format string for timestamp

ISSUE TYPE

New Module/Plugin Pull Request

COMPONENT NAME

timestamp

ADDITIONAL INFORMATION

Tested with:

ansible-test sanity --docker -v plugins/callback/timestamp.py
ansible-test integration --docker -v callback_timestamp

This is the first time for me to send PR for this collection. I have a few concerns and would love to hear any input. Thanks!

Is this plugin appropriate for community.general in the first place?
The timezone is set using ZoneInfo, which was added in Python 3.9. This means it will not work in Python 3.8. This collection requires Ansible 2.13 or higher, and Ansible 2.13 supports Python 3.8. However, since it is already EOL, this plugin has decided to ignore the user-provided timezone setting for Python 3.8 and below, simplifying the implementation. Is this a good practice?
About the test. I tried to write an integration test, but I cannot use the callback role like other callback plugins because the timestamp changes each time it is executed. For this reason I am only testing that the timestamp format is given a fixed string and that at least the timestamp is displayed in the proper position without the header line being folded back (this is the core feature of this plugin that has to be tested at least). Should I implement the test more rigorously?

If this should not continue, feel free to close it. Thanks in advance! 😃

kurokobo · 2024-05-05T14:28:11Z

Fixed issues for license and type annotation that caused CI to fail. Sorry for my trivial mistakes.
CI still fails but appears to be unrelated to my PR. Thanks😃

felixfontein

Thanks for your contribution!

I would probably add a seealso reference to the ansible.posix.profile_tasks callback (https://github.com/ansible-collections/ansible.posix/blob/main/plugins/callback/profile_tasks.py).

plugins/callback/timestamp.py

kurokobo · 2024-05-06T04:49:10Z

@felixfontein
Hi, thanks for your extremely rapid review! Corrections have been completed for all your comments. Please let me know if you have any other concerns.
Thanks! 😃

kurokobo · 2024-05-06T05:48:30Z

Sorry for massive updates, I've corrected invalid links (L() v.s. U()) and added seealso. Thanks!

kurokobo · 2024-05-07T03:52:06Z

@felixfontein
Thanks for working on this PR.
The EOL CI seems to have failed, but I believe this is intended since that adding any plugins to seealso is not yet supported in 2.13 and 2.14 of ansible-doc.

Any opinions on whether this failure should be ignored, or fixed in some way? Thanks!

plugins/callback/timestamp.py

felixfontein · 2024-05-07T05:17:25Z

The EOL CI seems to have failed, but I believe this is intended since that adding any plugins to seealso is not yet supported in 2.13 and 2.14 of ansible-doc.

Any opinions on whether this failure should be ignored, or fixed in some way? Thanks!

Please add ignore entries to tests/sanity/ignore-2.13.txt and tests/sanity/ignore-2.14.txt:

plugins/callback/timestamp.py validate-modules:invalid-documentation

(Please make sure to insert them alphabetically, i.e. before the first lookup.)

Co-authored-by: Felix Fontein <felix@fontein.de>

…codes

Co-authored-by: Felix Fontein <felix@fontein.de>

kurokobo · 2024-05-07T06:48:12Z

@felixfontein
Thanks for reviewiing this again, now I learned what ignore-* files are for.
Updated, and rebased (I believe rebasing is required to merge). Thanks!

felixfontein · 2024-05-07T20:09:41Z

Updated, and rebased (I believe rebasing is required to merge). Thanks!

Rebasing is not needed. It's only needed when you have conflicts, or you need code that's already in main but not yet in your PR's branch.

kurokobo · 2024-05-08T00:11:50Z

Aww, I was confusing the rules with other repositories, sorry for annoying you 😞

felixfontein · 2024-05-08T05:54:01Z

No worries! Rebasing doesn't hurt, and you definitely didn't annoy me :)

felixfontein

Looks good to me! If nobody objects, I'll merge this in ~a week.

russoz · 2024-05-12T05:47:04Z

LGTM

felixfontein · 2024-05-18T13:41:56Z

@kurokobo thanks for contributing this!
@russoz thanks for reviewing!

…r each header (ansible-collections#8308) * feat: add community.general.timestamp callback plugin * feat: add minimal integration tests for timestamp callback plugin * feat: add maintainers for timestamp callback plugin * fix: correct license * fix: remove type annotation for the older python environment * fix: remove unnecessary comment Co-authored-by: Felix Fontein <felix@fontein.de> * fix: add trailing period Co-authored-by: Felix Fontein <felix@fontein.de> * fix: split long description into list Co-authored-by: Felix Fontein <felix@fontein.de> * fix: remove default and add type Co-authored-by: Felix Fontein <felix@fontein.de> * fix; add type Co-authored-by: Felix Fontein <felix@fontein.de> * fix: split long description into list Co-authored-by: Felix Fontein <felix@fontein.de> * fix: improve description for format_string to describe usable format codes * fix: clarify the original codes and add copyright from that * fix: shorten long lines * fix: correct link format * fix: add seealso section * fix: add ignore entries for EOL CI * fix: update seealso to correctly associate with related plugin Co-authored-by: Felix Fontein <felix@fontein.de> --------- Co-authored-by: Felix Fontein <felix@fontein.de>

ansibullbot added callback callback plugin integration tests/integration new_contributor Help guide this first time contributor plugins plugin (any type) tests tests needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels May 5, 2024

felixfontein added the check-before-release PR will be looked at again shortly before release and merged if possible. label May 5, 2024

felixfontein reviewed May 5, 2024

View reviewed changes

ansibullbot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR label May 6, 2024

felixfontein reviewed May 7, 2024

View reviewed changes

plugins/callback/timestamp.py Outdated Show resolved Hide resolved

kurokobo and others added 15 commits May 7, 2024 15:46

feat: add community.general.timestamp callback plugin

0ce9140

feat: add minimal integration tests for timestamp callback plugin

2d82068

feat: add maintainers for timestamp callback plugin

c128cbb

fix: correct license

35771a5

fix: remove type annotation for the older python environment

6417fda

fix: remove unnecessary comment

fb8829e

Co-authored-by: Felix Fontein <felix@fontein.de>

fix: add trailing period

40820c9

Co-authored-by: Felix Fontein <felix@fontein.de>

fix: split long description into list

4481a94

Co-authored-by: Felix Fontein <felix@fontein.de>

fix: remove default and add type

1f27e19

Co-authored-by: Felix Fontein <felix@fontein.de>

fix; add type

89da321

Co-authored-by: Felix Fontein <felix@fontein.de>

fix: split long description into list

3de7a06

Co-authored-by: Felix Fontein <felix@fontein.de>

fix: improve description for format_string to describe usable format …

c826a91

…codes

fix: clarify the original codes and add copyright from that

30e79bb

fix: shorten long lines

0abf6b3

fix: correct link format

0fabe8b

kurokobo and others added 3 commits May 7, 2024 15:46

fix: add seealso section

5b32a1d

fix: add ignore entries for EOL CI

c439001

fix: update seealso to correctly associate with related plugin

3e226a0

Co-authored-by: Felix Fontein <felix@fontein.de>

kurokobo force-pushed the simple_timestamp branch from 29a1f60 to 3e226a0 Compare May 7, 2024 06:47

felixfontein approved these changes May 10, 2024

View reviewed changes

felixfontein merged commit bb73f28 into ansible-collections:main May 18, 2024
132 of 133 checks passed

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label May 18, 2024

kurokobo deleted the simple_timestamp branch May 18, 2024 13:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement timestamp callback plugin to show simple timestamp for each header #8308

feat: implement timestamp callback plugin to show simple timestamp for each header #8308

kurokobo commented May 5, 2024

kurokobo commented May 5, 2024

felixfontein left a comment •

edited

kurokobo commented May 6, 2024

kurokobo commented May 6, 2024

kurokobo commented May 7, 2024

felixfontein commented May 7, 2024

kurokobo commented May 7, 2024

felixfontein commented May 7, 2024

kurokobo commented May 8, 2024

felixfontein commented May 8, 2024

felixfontein left a comment

russoz commented May 12, 2024

felixfontein commented May 18, 2024

feat: implement timestamp callback plugin to show simple timestamp for each header #8308

feat: implement timestamp callback plugin to show simple timestamp for each header #8308

Conversation

kurokobo commented May 5, 2024

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

kurokobo commented May 5, 2024

felixfontein left a comment • edited

Choose a reason for hiding this comment

kurokobo commented May 6, 2024

kurokobo commented May 6, 2024

kurokobo commented May 7, 2024

felixfontein commented May 7, 2024

kurokobo commented May 7, 2024

felixfontein commented May 7, 2024

kurokobo commented May 8, 2024

felixfontein commented May 8, 2024

felixfontein left a comment

Choose a reason for hiding this comment

russoz commented May 12, 2024

felixfontein commented May 18, 2024

felixfontein left a comment •

edited