feat(list-versions): show cooldown status and upload timestamps#1127
feat(list-versions): show cooldown status and upload timestamps#1127shifa-khan wants to merge 1 commit intopython-wheel-build:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements the cooldown details phase of the release-age cooldown feature ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/fromager/commands/package.py (1)
196-245: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd per-requirement logging context in
list_versions
list_versionslogs requirement-specific messages but does not uselog.req_ctxvar_context(req), so contextual logging is inconsistent with other command paths.Suggested fix
- logger.info(f"Looking up versions for {req.name}") - if req.specifier: - logger.info(f"Filtering versions with specifier: {req.specifier}") + with log.req_ctxvar_context(req): + logger.info(f"Looking up versions for {req.name}") + if req.specifier: + logger.info(f"Filtering versions with specifier: {req.specifier}") - pbi = wkctx.package_build_info(req) - override_sdist_server_url = pbi.resolver_sdist_server_url(sdist_server_url) + pbi = wkctx.package_build_info(req) + override_sdist_server_url = pbi.resolver_sdist_server_url(sdist_server_url) - include_sdists, include_wheels = parse_distribution_option( - distribution_type, - pbi, - ) + include_sdists, include_wheels = parse_distribution_option( + distribution_type, + pbi, + ) - provider = overrides.find_and_invoke( + provider = overrides.find_and_invoke( ... - ) + ) + ...As per coding guidelines:
src/fromager/**/*.py: “Usereq_ctxvar_context()for per-requirement logging”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fromager/commands/package.py` around lines 196 - 245, The requirement-specific logs in this block should be executed inside the per-requirement logging context; wrap the code that starts after Requirement(requirement_spec) (the logger.info calls, pbi = wkctx.package_build_info(req), parse_distribution_option, overrides.find_and_invoke(...), provider.find_matches(...) and the candidates/versions handling) in the context manager returned by log.req_ctxvar_context(req) so that all requirement-scoped log messages use the req context; locate the code around Requirement(...) and wrap the subsequent logic (up through computing versions) with with log.req_ctxvar_context(req): to ensure consistent contextual logging.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fromager/commands/package.py`:
- Around line 263-270: The table branch ignores the requested output
destination: in the match on output_format you call
_export_versions_table(version_rows, req.name, cooldown) which always prints to
stdout, so update the call and the function to accept an output file/stream
(e.g. add an output parameter) and forward the provided output handle used by
the json/csv branches; modify _export_versions_table signature and all call
sites (including the other occurrence around lines 402-422) to accept and write
to the passed output instead of writing directly to stdout.
---
Outside diff comments:
In `@src/fromager/commands/package.py`:
- Around line 196-245: The requirement-specific logs in this block should be
executed inside the per-requirement logging context; wrap the code that starts
after Requirement(requirement_spec) (the logger.info calls, pbi =
wkctx.package_build_info(req), parse_distribution_option,
overrides.find_and_invoke(...), provider.find_matches(...) and the
candidates/versions handling) in the context manager returned by
log.req_ctxvar_context(req) so that all requirement-scoped log messages use the
req context; locate the code around Requirement(...) and wrap the subsequent
logic (up through computing versions) with with log.req_ctxvar_context(req): to
ensure consistent contextual logging.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 028ca9d6-ba8a-480e-8b7a-b9666d871f34
📒 Files selected for processing (2)
src/fromager/commands/package.pytests/test_list_versions.py
|
Tested the changes locally: $ fromager --min-release-age 20 package list-versions --details --format json click
13:52:27 INFO Looking up versions for click
13:52:27 INFO Found 57 version(s)
},
{
"package": "click",
"version": "8.3.0",
"upload_time": "2025-09-18T17:32:23.696258+00:00",
"age_days": "231",
"cooldown": "allowed"
},
{
"package": "click",
"version": "8.3.1",
"upload_time": "2025-11-15T20:45:42.706404+00:00",
"age_days": "172",
"cooldown": "allowed"
},
{
"package": "click",
"version": "8.3.2",
"upload_time": "2026-04-03T19:14:45.118083+00:00",
"age_days": "33",
"cooldown": "allowed"
},
{
"package": "click",
"version": "8.3.3",
"upload_time": "2026-04-22T15:11:27.506141+00:00",
"age_days": "15",
"cooldown": "blocked"
}
]
With a 20-day cooldown, click 8.3.3 (15 days old) is correctly marked as blocked while 8.3.2 (33 days old) is allowed. |
shifa-khan
force-pushed
the
list-versions-1078
branch
from
88f414f to
a967ee6
Compare
shifa-khan
force-pushed
the
list-versions-1078
branch
5 times, most recently
from
ba62e44 to
22685c7
Compare
| "--format", | ||
| "output_format", | ||
| type=click.Choice(["table", "csv", "json"], case_sensitive=False), | ||
| default="table", |
There was a problem hiding this comment.
What do you think about adding "requirements" to this list of options, and eliminating the --format-as-requirements flag? Then we could also add a format of "versions" and that would be the default, to maintain consistency. Then I think we can skip the --details flag entirely, because each format would tell us whether we should be including those details or not.
There was a problem hiding this comment.
Yeah, that makes sense. I was thinking about it too but since --format-as-requirements preexisted, I didn't want to make that change as part of this PR. I'll go ahead and fold it in.
There was a problem hiding this comment.
My first instinct would have been to preserve backwards compatibility, too, but I think in this case it's OK to fix things without doing that.
There was a problem hiding this comment.
yes, the deprecated list-versions shim still accepts --format-as-requirements and maps it to --format requirements
shifa-khan
force-pushed
the
list-versions-1078
branch
from
22685c7 to
39e1053
Compare
package list-versions now shows upload_time, age in days, and cooldown status for each version. --format controls the output: versions (default) and requirements print a filtered list excluding blocked entries; table, csv, and json include the full detail columns. This replaces the previous --details and --format-as-requirements flags. --ignore-per-package-overrides shows what the global --min-release-age would block without per-package exemptions. The deprecated list-versions shim maps --format-as-requirements to --format requirements for backward compat. Co-authored-by: Cursor <cursoragent@cursor.com>
shifa-khan
force-pushed
the
list-versions-1078
branch
from
39e1053 to
650cd2b
Compare
|
We need to add some e2e tests, it looks like. The version-only listing works properly, but the table formatter does not filter by age. |
2 participants
package list-versions now shows upload_time, age in days, and cooldown status for each version.
--format controls the output: versions (default) and requirements print a filtered list excluding blocked entries; table, csv, and json include the full detail columns. This replaces the previous --details and --format-as-requirements flags.
--ignore-per-package-overrides shows what the global --min-release-age would block without per-package exemptions. The deprecated list-versions shim maps --format-as-requirements to --format requirements for backward compat.
Closes: #1078