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

Refactor suite_report.py #35

Open
t00sa opened this issue Sep 3, 2024 · 2 comments
Open

Refactor suite_report.py #35

t00sa opened this issue Sep 3, 2024 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@t00sa
Copy link

t00sa commented Sep 3, 2024

This builds on the changes in #33 and refactors suite_report.py. The goals are to:

  • fix compliance with pylint as far as possible
  • improve code structure by
    • splitting the monolithic SuiteReport class
    • moving global variables into classes
    • separating trac table generation on a separate class
    • making internal methods private
  • use pathlib and associated functions in place of os.path and strings
  • improve/add google-style docstrings
  • update exception handling by
    • using appropriate builtin exceptions
    • removing broad except Exception catches
    • moving exception handling to the top level
    • using custom exceptions where necessary

The result should be a modernised version of the script which produces the same - or very similar - output as the unmodernised version.

@t00sa t00sa added the enhancement New feature or request label Sep 3, 2024
@t00sa t00sa self-assigned this Sep 3, 2024
@t00sa
Copy link
Author

t00sa commented Sep 3, 2024

Changes merged in from refactoring branch with a complete revision history. Hopefully this will make it a bit easier to review because the refactoring touches a lot of the code. With hindsight, this should've been split out into a series of tickets.

@t00sa
Copy link
Author

t00sa commented Sep 3, 2024

Nightly Testcases

Comparison between a summary generated with the refactored script and an one generated with the pre-issue #33 version, with a diff that ignores whitespace changes.

Set Revisions

The set-revisions shows a very minimal set of changes:

vdi> python suite_report.py -L /var/tmp -S ~frzz/cylc-run/um_set-revisions_nightly_2024-09-03/runN
vdi> diff -w /var/tmp/trac.log ~frzz/cylc-run/um_set-revisions_nightly_2024-09-03/runN/trac.log
7c7
<  || Report Generated: || 2024/09/03 14:22:52 || 
---
>  || Report Generated: || 2024/09/03 08:47:14 || 
11c11
<  || ''ROSE_ORIG_HOST'': || els055.metoffice.gov.uk || 
---
>  || ''ROSE_ORIG_HOST:'' || els055.metoffice.gov.uk || 
27a28
> 
38c39
<  || '''''' || '''Resource Monitoring Task''' ||
---
>  |||| '''Resource Monitoring Task''' || 
vdi> 

Heads

The results of a comparison using the heads nightly testing shows different revision numbers for some projects.

vdi> python suite_report.py -L /var/tmp -S ~frzz/cylc-run/um_heads_nightly_2024-09-03/runN
vdi> diff -w /var/tmp/trac.log ~frzz/cylc-run/um_heads_nightly_2024-09-03/runN/trac.log
7c7
<  || Report Generated: || 2024/09/03 14:25:30 || 
---
>  || Report Generated: || 2024/09/03 08:28:40 || 
11c11
<  || ''ROSE_ORIG_HOST'': || els055.metoffice.gov.uk || 
---
>  || ''ROSE_ORIG_HOST:'' || els055.metoffice.gov.uk || 
14,15c14,15
<  || CASIM ||  fcm:casim.xm_tr@HEAD  ||  [https://code.metoffice.gov.uk/trac/monc/browser/casim/trunk?rev=11389 fcm:casim.x_tr@HEAD]  ||  ||  ||  ||
<  || JULES ||  fcm:jules.xm_tr@HEAD  ||  [https://code.metoffice.gov.uk/trac/jules/browser/main/trunk?rev=28870 fcm:jules.x_tr@HEAD]  ||  ||  ||  ||
---
>  || CASIM ||  fcm:casim.xm_tr@HEAD ||  [https://code.metoffice.gov.uk/trac/monc/browser/casim/trunk?rev=11387 fcm:casim.x_tr@HEAD] ||  ||  ||  || 
>  || JULES ||  fcm:jules.xm_tr@HEAD ||  [https://code.metoffice.gov.uk/trac/jules/browser/main/trunk?rev=28867 fcm:jules.x_tr@HEAD] ||  ||  ||  || 
17c17
<  || MULE ||  fcm:mule.xm_tr@HEAD  ||  [https://code.metoffice.gov.uk/trac/um/browser/mule/trunk?rev=125861 fcm:mule.x_tr@HEAD]  ||  ||  ||  ||
---
>  || MULE ||  fcm:mule.xm_tr@HEAD ||  [https://code.metoffice.gov.uk/trac/um/browser/mule/trunk?rev=125856 fcm:mule.x_tr@HEAD] ||  ||  ||  || 
20c20
<  || UKCA ||  fcm:ukca.xm_tr@HEAD  ||  [https://code.metoffice.gov.uk/trac/ukca/browser/main/trunk?rev=4568 fcm:ukca.x_tr@HEAD]  ||  ||  ||  ||
---
>  || UKCA ||  fcm:ukca.xm_tr@HEAD ||  [https://code.metoffice.gov.uk/trac/ukca/browser/main/trunk?rev=4566 fcm:ukca.x_tr@HEAD] ||  ||  ||  || 
27a28
> 
38c39
<  || '''''' || '''Resource Monitoring Task''' ||
---
>  |||| '''Resource Monitoring Task''' || 
vdi> 

These differences are probably the result of a bug in the existing version of suite_report.py, which uses fcm loc-layout to get the heads revision from the peg_revision. Where the target is the head of the trunk, the peg_revision ID refers to the most recent change on the repository, not the most recent change to the trunk. Consequently, if a new branch has been cut since the trac.log was generated by the nightly testing and before the new report was generated, the revision IDs in the URLs will be different (and wrong) in each case.

This problem is marked with a # FIXME note. Resolution probably requires input from someone with a better understanding of fcm branches and versioning.

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

When branches are created from issues, their pull requests are automatically linked.

1 participant