Improve CVE metrics report format#464
Conversation
Assisted-by: Claude Opus 4.6
There was a problem hiding this comment.
Code Review
This pull request enhances the ymir_cve_metrics.py script by replacing the text-based summary with a Markdown report that includes clickable links to GitLab merge requests and Jira issues. It also refines the GitLab MR pagination logic to handle date drift for merged items by relying on in-code filtering for the upper bound. Feedback focuses on improving the accuracy and robustness of the generated URLs, specifically by supporting multiple authors in GitLab search links, ensuring date filters are applied to open MR links to match the reported counts, and addressing potential URL length limitations when linking large sets of Jira keys.
| # GitLab web UI only supports a single author_username filter. | ||
| base = "https://gitlab.com/groups/redhat/-/merge_requests" | ||
| params = f"?state={state}&author_username={authors[0]}&first_page_size=20" |
There was a problem hiding this comment.
The comment states that the GitLab web UI only supports a single author filter, but it actually supports multiple authors using the author_username[] array syntax (e.g., ?author_username[]=user1&author_username[]=user2). Since the script allows multiple authors via the --gitlab-author CLI argument, the generated links should include all of them to accurately reflect the data in the report.
# GitLab web UI supports multiple author_username filters using array syntax.
base = "https://gitlab.com/groups/redhat/-/merge_requests"
author_params = "&".join(f"author_username[]={quote(a)}" for a in authors)
params = f"?state={state}&{author_params}&first_page_size=20"| elif state == "opened": | ||
| params += "&sort=updated_desc" |
There was a problem hiding this comment.
The "Open MRs" link in the report does not include date filters, whereas the count reported (non_merged_mrs) is filtered by the created_at date range in main(). This leads to a discrepancy where the link shows all open MRs by the author, but the text indicates a specific count for the period. Adding created_after and created_before parameters to the URL would ensure consistency.
| elif state == "opened": | |
| params += "&sort=updated_desc" | |
| elif state == "opened": | |
| params += "&sort=updated_desc" | |
| if date_from and date_to: | |
| params += ( | |
| f"&created_after={date_from.strftime('%Y-%m-%d')}" | |
| f"&created_before={date_to.strftime('%Y-%m-%d')}" | |
| ) |
There was a problem hiding this comment.
these parameters dont work with web UI
| keys_csv = ",".join(sorted(jira_keys)) | ||
| jql = quote(f"issuekey in ({keys_csv})", safe="") | ||
| return f"{jira_url.rstrip('/')}/issues/?jql={jql}" |
There was a problem hiding this comment.
Generating a Jira search URL by joining a large number of issue keys in a JQL in (...) clause can result in extremely long URLs. Most browsers and servers have limits on URL length (typically around 2,000 to 8,000 characters). If the report covers a very active period with hundreds of Jiras, these links might be truncated or rejected by the server. Consider adding a check or a warning if the number of keys is exceptionally high.
TomasTomecek
left a comment
There was a problem hiding this comment.
thank you, very nice improvement!
| keys_csv = ",".join(sorted(jira_keys)) | ||
| jql = quote(f"issuekey in ({keys_csv})", safe="") | ||
| return f"{jira_url.rstrip('/')}/issues/?jql={jql}" |
- Report numbers now link to Jira JQL searches (resolved, closed, pending) and GitLab MR searches (merged with date range, open) - Extract Jira keys from open MRs to link pending issues - Fix merged MR undercounting by dropping updated_at upper bound constraint that excluded MRs with post-merge activity Assisted-by: Claude Opus 4.6
Categorise MRs by ymir_backport, ymir_rebuild, ymir_rebase labels and show inline summary, e.g. "3 MRs — 2 backport, 1 rebuild". Assisted-by: Claude Opus 4.6
83414a2 to
55a3424
Compare
| # GitLab API filter params are created_after/updated_after (no _at), | ||
| # but order_by uses created_at/updated_at. | ||
| filter_prefix = date_field.removesuffix("_at") |
There was a problem hiding this comment.
lol gitlab, why not make things easy right
Example output:
Ymir CVE Activity Report: 2026-05-04 → 2026-05-11
Solved Jiras
In Progress Jiras
Follow-up: