[WINC-1850] Automate determining z-stream releases #3996
[WINC-1850] Automate determining z-stream releases #3996wgahnagl wants to merge 1 commit intoopenshift:masterfrom
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
📝 WalkthroughWalkthroughA new Python module 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wgahnagl The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
hack/release-automater/release_overview.py (2)
353-359: Renameltolightnessin HSL helpers for clarity.The single-letter
lis easy to misread and is flagged by linting.Suggested fix
def desaturate_hsl(hsl, factor=0.5, darken=0.85): - h, s, l = hsl - return (h, s * factor, l * darken) + h, s, lightness = hsl + return (h, s * factor, lightness * darken) def hsl_to_hex(hsl): - h, s, l = hsl - r, g, b = colorsys.hls_to_rgb(h / 360.0, l, s) + h, s, lightness = hsl + r, g, b = colorsys.hls_to_rgb(h / 360.0, lightness, s)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/release-automater/release_overview.py` around lines 353 - 359, Rename the HSL helper variable `l` to `lightness` to improve clarity and satisfy linting: update the parameter unpacking and all uses in the functions desaturate_hsl and hsl_to_hex (change "h, s, l = hsl" to "h, s, lightness = hsl", and replace subsequent uses of `l` with `lightness`, including the call to colorsys.hls_to_rgb and the return tuple in desaturate_hsl); ensure function signatures/returns remain the same semantically.
17-216: Add short docstrings for key functions (summarize_versions, fetchers, and runners).Given this is a reusable hack script with both CLI and GUI paths, minimal docstrings will make maintenance and handoff easier.
As per coding guidelines,
hack/**/*: Review development and build scripts:Check for proper documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/release-automater/release_overview.py` around lines 17 - 216, Add concise one-line docstrings to the key functions to satisfy the documentation guideline: add short descriptions for summarize_versions (what it returns), the fetchers get_lifecycle_data and get_grade_data (what API they call and what they return), extract_grade_data (what structure it transforms and returns), and the runners main and run_cli (how they dispatch CLI/GUI and return type); include brief notes on important parameters/returns (e.g., summarize_versions returns a list of version summaries, extract_grade_data returns Dict[str, List[Dict]] keyed by minor version) and place each docstring immediately below the corresponding def (e.g., def summarize_versions():, def get_lifecycle_data():, def get_grade_data():, def extract_grade_data(...):, def main():, def run_cli():) using a minimal single-line or short triple-quoted string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hack/release-automater/release_overview.py`:
- Around line 270-271: The grid column configuration loop currently configures
only five columns (for i in range(5)) while the table renders six columns
including the action/RELEASE column; update the code that calls
status_frame.grid_columnconfigure (the loop) to include column index 5 (e.g.,
change range(5) to range(6) or add a call status_frame.grid_columnconfigure(5,
weight=1)) so the RELEASE/action column is given weight and resizes
consistently.
- Line 117: The requests.get calls in release_overview.py (the statement res =
requests.get(url, headers=headers, params=params) and the two other outbound
HTTP calls in this file) are missing timeouts; update each call site to pass an
explicit timeout=15 parameter (e.g., requests.get(..., timeout=15)) and ensure
any surrounding try/except logic will handle
requests.exceptions.Timeout/RequestException consistently (log and fail fast) so
the release automation cannot hang indefinitely.
- Around line 311-340: The RELEASE tk.Button is being created inside the inner
loop that iterates over values (for col, value in enumerate(values)), causing
five redundant buttons per row; move the tk.Button creation (the block that
calls tk.Button(..., command=lambda vname=v["version"]: release_version(vname),
...).grid(...)) out of that inner loop so it executes once after the label loop
completes for each row (i.e., unindent it to the same level as the for col,
value in enumerate(values) loop body), keeping status_frame, release_version,
and v["version"] usage unchanged.
- Line 232: The call to v["end_of_life"].strftime(...) can raise AttributeError
when get_eol_date() returns None; update the formatting at the places that
consume the data (where v["end_of_life"] is used, e.g., the strftime calls at
the spots flagged) to guard against None (for example, check if
v.get("end_of_life") is truthy before calling strftime and emit a default like
"" or "N/A" when missing). Locate the data producer get_eol_date and the
consumer code that formats v["end_of_life"] (the two strftime usages) and add
the conditional/ternary-style handling so None values are safely serialized
instead of calling strftime on them.
- Around line 79-95: The code currently drops entire versions when parsing fails
due to bare excepts around computing minor_version and fetching grade info;
update the logic in the loop that reads version["name"] and accesses grade_data
so it never continues on exceptions but instead sets safe defaults (e.g.,
minor_version = "unknown", wmco_z_stream_version = "unknown", latest_grade =
"unknown"). Replace the first try-except around minor_version with a guarded
parse using version.get("name") and split only if present, falling back to
"unknown"; replace the second try-except that indexes grade_data with a safe
lookup using grade_data.get(f"10.{minor_version}", []) and then handle an empty
list by leaving wmco_z_stream_version and latest_grade as "unknown". Ensure the
code still appends/records the version entry even when fields are unknown so no
versions are omitted.
- Around line 22-23: The bare except in to_date() swallows system-level
exceptions; replace it with explicit exception handling by catching only
parsing-related errors (e.g., except (ValueError, TypeError) as e:) and return
None (or log e) there, and let other exceptions like KeyboardInterrupt and
SystemExit propagate (do not catch them). Locate the to_date() function and
change the bare "except:" to an explicit tuple of exceptions so only parse/type
errors are handled.
- Around line 210-214: main() currently calls get_lifecycle_data() and
get_grade_data() (indirectly via run_cli()/run_gui()) without handling
network/API failures; wrap the API call sequence in a try/except that catches
requests.RequestException (or a broad Exception as a fallback), log a clear
error message including exception details, and exit with a non‑zero status (e.g.
return/raise SystemExit with code != 0) so automation pipelines see failure;
update the code paths around run_cli()/run_gui() and the top-level return logic
to ensure exceptions from get_lifecycle_data()/get_grade_data() are caught and
translated to a logged error + nonzero exit instead of an unhandled traceback.
---
Nitpick comments:
In `@hack/release-automater/release_overview.py`:
- Around line 353-359: Rename the HSL helper variable `l` to `lightness` to
improve clarity and satisfy linting: update the parameter unpacking and all uses
in the functions desaturate_hsl and hsl_to_hex (change "h, s, l = hsl" to "h, s,
lightness = hsl", and replace subsequent uses of `l` with `lightness`, including
the call to colorsys.hls_to_rgb and the return tuple in desaturate_hsl); ensure
function signatures/returns remain the same semantically.
- Around line 17-216: Add concise one-line docstrings to the key functions to
satisfy the documentation guideline: add short descriptions for
summarize_versions (what it returns), the fetchers get_lifecycle_data and
get_grade_data (what API they call and what they return), extract_grade_data
(what structure it transforms and returns), and the runners main and run_cli
(how they dispatch CLI/GUI and return type); include brief notes on important
parameters/returns (e.g., summarize_versions returns a list of version
summaries, extract_grade_data returns Dict[str, List[Dict]] keyed by minor
version) and place each docstring immediately below the corresponding def (e.g.,
def summarize_versions():, def get_lifecycle_data():, def get_grade_data():, def
extract_grade_data(...):, def main():, def run_cli():) using a minimal
single-line or short triple-quoted string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 5faf9500-d884-4f28-8cf7-58c8f65d7e03
📒 Files selected for processing (1)
hack/release-automater/release_overview.py
| except: | ||
| return None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify no bare except remains in this script
rg -nP '^\s*except\s*:\s*$' hack/release-automater/release_overview.pyRepository: openshift/windows-machine-config-operator
Length of output: 137
🏁 Script executed:
cat -n hack/release-automater/release_overview.py | head -100Repository: openshift/windows-machine-config-operator
Length of output: 3436
🏁 Script executed:
# Check imports and function signature around to_date()
rg -B5 -A5 'def to_date' hack/release-automater/release_overview.pyRepository: openshift/windows-machine-config-operator
Length of output: 344
Replace bare except in to_date() with explicit exception handling.
Line 22 catches everything indiscriminately—including KeyboardInterrupt and SystemExit—making it impossible to distinguish parse failures from system-level errors. This violates proper error-handling standards for infrastructure automation.
Suggested fix
-from dateutil.parser import parse
+from dateutil.parser import ParserError, parse
...
- except:
+ except (ParserError, TypeError, ValueError):
return None🧰 Tools
🪛 Ruff (0.15.10)
[error] 22-22: Do not use bare except
(E722)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/release-automater/release_overview.py` around lines 22 - 23, The bare
except in to_date() swallows system-level exceptions; replace it with explicit
exception handling by catching only parsing-related errors (e.g., except
(ValueError, TypeError) as e:) and return None (or log e) there, and let other
exceptions like KeyboardInterrupt and SystemExit propagate (do not catch them).
Locate the to_date() function and change the bare "except:" to an explicit tuple
of exceptions so only parse/type errors are handled.
| try: | ||
| minor_version = version["name"].split('.')[1] | ||
| except: | ||
| continue | ||
|
|
||
| try: | ||
| grade_info_list = grade_data[f"10.{minor_version}"] | ||
| most_recent = grade_info_list[0] # First element is most recent z-stream | ||
|
|
||
| wmco_z_stream_version = most_recent['version'] | ||
|
|
||
| grades = most_recent.get('freshness_grades', []) | ||
| if grades: | ||
| latest_grade = grades[0].get('grade') if isinstance(grades[0], dict) else grades[0] | ||
|
|
||
| except: | ||
| continue |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "release_overview.py" -type fRepository: openshift/windows-machine-config-operator
Length of output: 129
🏁 Script executed:
cat -n hack/release-automater/release_overview.py | head -120 | tail -60Repository: openshift/windows-machine-config-operator
Length of output: 2266
🏁 Script executed:
head -30 hack/release-automater/release_overview.pyRepository: openshift/windows-machine-config-operator
Length of output: 723
🏁 Script executed:
cat -n hack/release-automater/release_overview.py | sed -n '50,120p'Repository: openshift/windows-machine-config-operator
Length of output: 2662
Include versions with partial or missing data instead of silently dropping them.
Lines 82 and 95 use bare except: continue statements that omit entire versions from the report when parsing fails or grade data is unavailable. Since this script builds a release overview, every version should appear in the output—with defaults ("unknown") for missing fields. This preserves visibility into the complete release landscape and prevents gaps from going unnoticed.
Replace the try-except blocks with safer lookups using .get() and graceful fallbacks:
Suggested fix
- try:
- minor_version = version["name"].split('.')[1]
- except:
- continue
+ parts = version.get("name", "").split(".")
+ minor_version = parts[1] if len(parts) > 1 else None
- try:
- grade_info_list = grade_data[f"10.{minor_version}"]
+ grade_info_list = grade_data.get(f"10.{minor_version}", []) if minor_version else []
+ if grade_info_list:
most_recent = grade_info_list[0] # First element is most recent z-stream
wmco_z_stream_version = most_recent['version']
grades = most_recent.get('freshness_grades', [])
if grades:
latest_grade = grades[0].get('grade') if isinstance(grades[0], dict) else grades[0]
-
- except:
- continue🧰 Tools
🪛 Ruff (0.15.10)
[error] 81-81: Do not use bare except
(E722)
[error] 81-82: try-except-continue detected, consider logging the exception
(S112)
[error] 94-94: Do not use bare except
(E722)
[error] 94-95: try-except-continue detected, consider logging the exception
(S112)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/release-automater/release_overview.py` around lines 79 - 95, The code
currently drops entire versions when parsing fails due to bare excepts around
computing minor_version and fetching grade info; update the logic in the loop
that reads version["name"] and accesses grade_data so it never continues on
exceptions but instead sets safe defaults (e.g., minor_version = "unknown",
wmco_z_stream_version = "unknown", latest_grade = "unknown"). Replace the first
try-except around minor_version with a guarded parse using version.get("name")
and split only if present, falling back to "unknown"; replace the second
try-except that indexes grade_data with a safe lookup using
grade_data.get(f"10.{minor_version}", []) and then handle an empty list by
leaving wmco_z_stream_version and latest_grade as "unknown". Ensure the code
still appends/records the version entry even when fields are unknown so no
versions are omitted.
| "Accept": "application/json" | ||
| } | ||
|
|
||
| res = requests.get(url, headers=headers, params=params) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all requests/session GET call sites for timeout coverage
rg -nP '(requests\.get|session\.get)\(' hack/release-automater/release_overview.pyRepository: openshift/windows-machine-config-operator
Length of output: 219
Add explicit timeouts to all outbound HTTP calls to prevent automation hangs.
Lines 117, 128, and 141 make HTTP requests without timeout parameters. Network calls that hang indefinitely will stall the release automation pipeline—this is a critical error handling gap.
All three call sites need explicit timeout configuration. A 15-second timeout is reasonable for catalog API interactions.
Suggested fix
REPO_ID = "5fb813954070f53cd79231ff"
PYXIS_API = "https://catalog.redhat.com/api/containers/v1"
+HTTP_TIMEOUT_SECONDS = 15
...
- res = requests.get(url, headers=headers, params=params)
+ res = requests.get(url, headers=headers, params=params, timeout=HTTP_TIMEOUT_SECONDS)
...
- resp = session.get(url)
+ resp = session.get(url, timeout=HTTP_TIMEOUT_SECONDS)
...
- resp = session.get(images_url)
+ resp = session.get(images_url, timeout=HTTP_TIMEOUT_SECONDS)🧰 Tools
🪛 Ruff (0.15.10)
[error] 117-117: Probable use of requests call without timeout
(S113)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/release-automater/release_overview.py` at line 117, The requests.get
calls in release_overview.py (the statement res = requests.get(url,
headers=headers, params=params) and the two other outbound HTTP calls in this
file) are missing timeouts; update each call site to pass an explicit timeout=15
parameter (e.g., requests.get(..., timeout=15)) and ensure any surrounding
try/except logic will handle requests.exceptions.Timeout/RequestException
consistently (log and fail fast) so the release automation cannot hang
indefinitely.
| if args.gui: | ||
| run_gui() | ||
| else: | ||
| return run_cli() | ||
| return 0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the file
fd -t f release_overview.py hack/Repository: openshift/windows-machine-config-operator
Length of output: 127
🏁 Script executed:
# Once found, examine the relevant section and surrounding context
cat -n hack/release-automater/release_overview.py | head -250 | tail -100Repository: openshift/windows-machine-config-operator
Length of output: 3572
🏁 Script executed:
find . -name "release_overview.py" -type f 2>/dev/nullRepository: openshift/windows-machine-config-operator
Length of output: 129
🏁 Script executed:
# Check imports and see what modules are used
head -50 hack/release-automater/release_overview.pyRepository: openshift/windows-machine-config-operator
Length of output: 1165
🏁 Script executed:
# Check the summarize_versions function and other functions that might make API calls
sed -n '1,150p' hack/release-automater/release_overview.py | head -100Repository: openshift/windows-machine-config-operator
Length of output: 2736
🏁 Script executed:
# Look for any requests or API calls in the file
rg "requests\.|http\.|api|API" hack/release-automater/release_overview.py -n -A 2Repository: openshift/windows-machine-config-operator
Length of output: 1071
🏁 Script executed:
# Look at get_lifecycle_data and get_grade_data functions to see if they handle exceptions
sed -n '100,145p' hack/release-automater/release_overview.pyRepository: openshift/windows-machine-config-operator
Length of output: 1432
🏁 Script executed:
# Check if there's any try-catch in run_gui and run_cli
sed -n '200,280p' hack/release-automater/release_overview.pyRepository: openshift/windows-machine-config-operator
Length of output: 1895
🏁 Script executed:
# Check the complete main function and surrounding context to see if script is called with any error handling
tail -20 hack/release-automater/release_overview.pyRepository: openshift/windows-machine-config-operator
Length of output: 506
🏁 Script executed:
# Check if there's any try-catch around summarize_versions calls in run_cli or run_gui
rg "try|except|raise" hack/release-automater/release_overview.py -n -B 2 -A 2Repository: openshift/windows-machine-config-operator
Length of output: 1728
Add error handling for API failures in main().
The script makes HTTP requests to external APIs (get_lifecycle_data() and get_grade_data()) without catching failures. Network timeouts, connection errors, and HTTP error responses propagate as unhandled exceptions, producing noisy tracebacks unsuitable for automation pipelines. This violates proper error handling practices for build/release tooling.
Suggested fix
def main():
parser = argparse.ArgumentParser(description="WMCO Release Status Dashboard")
parser.add_argument(
"-g", "--gui",
action="store_true",
help="Run using GUI"
)
args = parser.parse_args()
- if args.gui:
- run_gui()
- else:
- return run_cli()
- return 0
+ try:
+ if args.gui:
+ run_gui()
+ else:
+ run_cli()
+ return 0
+ except requests.RequestException as exc:
+ print(f"request failed: {exc}", file=sys.stderr)
+ return 2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/release-automater/release_overview.py` around lines 210 - 214, main()
currently calls get_lifecycle_data() and get_grade_data() (indirectly via
run_cli()/run_gui()) without handling network/API failures; wrap the API call
sequence in a try/except that catches requests.RequestException (or a broad
Exception as a fallback), log a clear error message including exception details,
and exit with a non‑zero status (e.g. return/raise SystemExit with code != 0) so
automation pipelines see failure; update the code paths around
run_cli()/run_gui() and the top-level return logic to ensure exceptions from
get_lifecycle_data()/get_grade_data() are caught and translated to a logged
error + nonzero exit instead of an unhandled traceback.
| rows.append([ | ||
| v["version"], | ||
| v["current_phase"] or "EOL", | ||
| v["end_of_life"].strftime("%Y-%m-%d"), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd hack/release-automater && wc -l release_overview.pyRepository: openshift/windows-machine-config-operator
Length of output: 108
🏁 Script executed:
cd hack/release-automater && sed -n '220,245p' release_overview.pyRepository: openshift/windows-machine-config-operator
Length of output: 703
🏁 Script executed:
cd hack/release-automater && sed -n '295,315p' release_overview.pyRepository: openshift/windows-machine-config-operator
Length of output: 662
🏁 Script executed:
cd hack/release-automater && grep -n "def get_eol_date" release_overview.pyRepository: openshift/windows-machine-config-operator
Length of output: 114
🏁 Script executed:
cd hack/release-automater && sed -n '25,80p' release_overview.pyRepository: openshift/windows-machine-config-operator
Length of output: 1608
🏁 Script executed:
cd hack/release-automater && grep -n "end_of_life" release_overview.py | head -20Repository: openshift/windows-machine-config-operator
Length of output: 236
Guard end_of_life before calling strftime().
The get_eol_date() function (line 25) can legitimately return None for incomplete lifecycle data, and this None value propagates directly into the data dictionary at line 100. Lines 232 and 306 will throw AttributeError when attempting to call .strftime() on None.
Suggested fix
- v["end_of_life"].strftime("%Y-%m-%d"),
+ v["end_of_life"].strftime("%Y-%m-%d") if v["end_of_life"] else "N/A",
...
- v["end_of_life"].strftime("%B %d, %Y"),
+ v["end_of_life"].strftime("%B %d, %Y") if v["end_of_life"] else "N/A",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/release-automater/release_overview.py` at line 232, The call to
v["end_of_life"].strftime(...) can raise AttributeError when get_eol_date()
returns None; update the formatting at the places that consume the data (where
v["end_of_life"] is used, e.g., the strftime calls at the spots flagged) to
guard against None (for example, check if v.get("end_of_life") is truthy before
calling strftime and emit a default like "" or "N/A" when missing). Locate the
data producer get_eol_date and the consumer code that formats v["end_of_life"]
(the two strftime usages) and add the conditional/ternary-style handling so None
values are safely serialized instead of calling strftime on them.
| for i in range(5): | ||
| status_frame.grid_columnconfigure(i, weight=1) |
There was a problem hiding this comment.
Configure all grid columns used by the table.
You render 6 columns (including the action column) but configure only 5. Include column 5 so the RELEASE column resizes consistently.
Suggested fix
- for i in range(5):
+ for i in range(6):
status_frame.grid_columnconfigure(i, weight=1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for i in range(5): | |
| status_frame.grid_columnconfigure(i, weight=1) | |
| for i in range(6): | |
| status_frame.grid_columnconfigure(i, weight=1) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/release-automater/release_overview.py` around lines 270 - 271, The grid
column configuration loop currently configures only five columns (for i in
range(5)) while the table renders six columns including the action/RELEASE
column; update the code that calls status_frame.grid_columnconfigure (the loop)
to include column index 5 (e.g., change range(5) to range(6) or add a call
status_frame.grid_columnconfigure(5, weight=1)) so the RELEASE/action column is
given weight and resizes consistently.
| for col, value in enumerate(values): | ||
| tk.Label( | ||
| status_frame, | ||
| text=value, | ||
| bg=bg, | ||
| fg="black", | ||
| anchor="w", | ||
| bd=0, | ||
| highlightthickness=0, | ||
| padx=8, | ||
| pady=6, | ||
| ).grid( | ||
| row=row, | ||
| column=col, | ||
| sticky="nsew", | ||
| padx=0, | ||
| pady=0 | ||
| ) | ||
| tk.Button( | ||
| status_frame, | ||
| text="RELEASE", | ||
| command=lambda vname=v["version"]: release_version(vname), | ||
| bg="#444", | ||
| fg="white", | ||
| activebackground="#666", | ||
| activeforeground="white", | ||
| bd=0, | ||
| highlightthickness=0, | ||
| cursor="hand2", | ||
| ).grid(row=row, column=5, sticky="nsew", padx=0, pady=0) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n hack/release-automater/release_overview.py | sed -n '300,350p'Repository: openshift/windows-machine-config-operator
Length of output: 1873
🏁 Script executed:
cat -n hack/release-automater/release_overview.py | sed -n '280,345p'Repository: openshift/windows-machine-config-operator
Length of output: 2330
Move RELEASE button outside the inner loop—it's being created redundantly on each iteration.
The button instantiation at lines 329–340 sits inside the for col, value in enumerate(values) loop, which runs 5 times per row. This creates 5 button widgets at the same grid position (row=row, column=5), with only the final one visible. The grid manager keeps overwriting earlier instances.
Unindent the button block to place it after the label loop completes—it belongs at the outer for row, v in enumerate(data, start=1) indentation level. This gives you one button per row, properly positioned at column 5.
Suggested fix
for col, value in enumerate(values):
tk.Label(
status_frame,
@@
).grid(
row=row,
column=col,
sticky="nsew",
padx=0,
pady=0
)
- tk.Button(
- status_frame,
- text="RELEASE",
- command=lambda vname=v["version"]: release_version(vname),
- bg="#444",
- fg="white",
- activebackground="#666",
- activeforeground="white",
- bd=0,
- highlightthickness=0,
- cursor="hand2",
- ).grid(row=row, column=5, sticky="nsew", padx=0, pady=0)
+ tk.Button(
+ status_frame,
+ text="RELEASE",
+ command=lambda vname=v["version"]: release_version(vname),
+ bg="#444",
+ fg="white",
+ activebackground="#666",
+ activeforeground="white",
+ bd=0,
+ highlightthickness=0,
+ cursor="hand2",
+ ).grid(row=row, column=5, sticky="nsew", padx=0, pady=0)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for col, value in enumerate(values): | |
| tk.Label( | |
| status_frame, | |
| text=value, | |
| bg=bg, | |
| fg="black", | |
| anchor="w", | |
| bd=0, | |
| highlightthickness=0, | |
| padx=8, | |
| pady=6, | |
| ).grid( | |
| row=row, | |
| column=col, | |
| sticky="nsew", | |
| padx=0, | |
| pady=0 | |
| ) | |
| tk.Button( | |
| status_frame, | |
| text="RELEASE", | |
| command=lambda vname=v["version"]: release_version(vname), | |
| bg="#444", | |
| fg="white", | |
| activebackground="#666", | |
| activeforeground="white", | |
| bd=0, | |
| highlightthickness=0, | |
| cursor="hand2", | |
| ).grid(row=row, column=5, sticky="nsew", padx=0, pady=0) | |
| for col, value in enumerate(values): | |
| tk.Label( | |
| status_frame, | |
| text=value, | |
| bg=bg, | |
| fg="black", | |
| anchor="w", | |
| bd=0, | |
| highlightthickness=0, | |
| padx=8, | |
| pady=6, | |
| ).grid( | |
| row=row, | |
| column=col, | |
| sticky="nsew", | |
| padx=0, | |
| pady=0 | |
| ) | |
| tk.Button( | |
| status_frame, | |
| text="RELEASE", | |
| command=lambda vname=v["version"]: release_version(vname), | |
| bg="#444", | |
| fg="white", | |
| activebackground="#666", | |
| activeforeground="white", | |
| bd=0, | |
| highlightthickness=0, | |
| cursor="hand2", | |
| ).grid(row=row, column=5, sticky="nsew", padx=0, pady=0) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/release-automater/release_overview.py` around lines 311 - 340, The
RELEASE tk.Button is being created inside the inner loop that iterates over
values (for col, value in enumerate(values)), causing five redundant buttons per
row; move the tk.Button creation (the block that calls tk.Button(...,
command=lambda vname=v["version"]: release_version(vname), ...).grid(...)) out
of that inner loop so it executes once after the label loop completes for each
row (i.e., unindent it to the same level as the for col, value in
enumerate(values) loop body), keeping status_frame, release_version, and
v["version"] usage unchanged.
|
@wgahnagl: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
This script allows for getting a quick overview of the health of all released wmco versions, their current z-stream, and helps to determine if a new z-stream release is needed.
Summary by CodeRabbit