cli: add --min-release-age for relative update delay#13837
cli: add --min-release-age for relative update delay#13837
Conversation
|
Before I review the code can you expand on the design choices you made here given the discussion in #13674, let us know if you purposely made a choice and why, or if it was arbitrary:
|
|
Thanks. I answered the questions below. I can see how this could be simplified, and look forward to feedback. Why did you create a new option rather than using the existing --uploaded-prior-to option?The parameter name was chosen from the (p)npm tool for JS: --min-release-age 2, where plain integer translates to days. I would like to have it consistent across tools, to make it simple for users. --uploaded-prior-to maps more to the --before, and I related both to an absolute time stamp. Why did you create new unitsThe units are inspired by the "friendly" duration from "uv". The pnpm tool also has a minimumReleaseAge. While npm uses days as unit, pnpm uses to minutes. To make the input more human readable, and to support different time precision, I started with the user friendly units. I am not sure whether many users would like the ISO duration that is discussed in #13674. npm has several pull requests that have a simple unit. |
|
I recommend reading to the end of #13674, there is a lot of design discussion there and seems like we were heading towards consensus about what to support. I don't think we should consider JS or other ecosystems above the Python ecosystem, so going forward with the option pip has selected and a subset of the format(s) that uv has chosen seems like it'd be the most consistent experience for Python users. |
|
In the issue, to me there is no consensus on the details yet. I asked in that issue as well. The parameter supports plain integers as days, or units similar to uv's "friendly" format. I decided not to overload "--uploaded-prior-to", as that parameter implies an absolute time stamp and I find "--uploaded-prior-to=3days" misleading. It is not clear to me whether you already agreed on overloading "--uploaded-prior-to" or agree to have a separate parameter. I can change the parameter name to "exclude-newer", if consistency to "uv" is more important than to npm. For the implementation, could we go with an incremental approach and add the parameter with an integer unit for e.g. days first, and later add support for strings with further units based on user feedback? |
nmanthey
force-pushed
the
min_release_age
branch
from
e1aa2ee to
bd9b1bc
Compare
|
I changed the logic to extend the existing parameter Testing DoneUsing pip with the --uploaded-prior-to and the value given in the test case, to then show the installed version of the certifi package. |
notatallshaw
left a comment
notatallshaw
left a comment
There was a problem hiding this comment.
This needs a section in the user guide to carefully reflect how to communicate this following the discussion: #13674
It would come in the "Filtering By Upload Time" section https://pip.pypa.io/en/stable/user_guide/#filtering-by-upload-time
nmanthey
force-pushed
the
min_release_age
branch
from
bd9b1bc to
7ae71ef
Compare
src/pip/_internal/cli/cmdoptions.py
Outdated
| # to disambiguate from absolute datetimes. Only whole days are supported; | ||
| # the format may be extended to more of the ISO 8601 duration syntax in | ||
| # the future if a real need is presented. | ||
| match = re.match(r"^P(\d+)D$", value, re.IGNORECASE) |
There was a problem hiding this comment.
I believe ISO-8601 duration designators are uppercase only, please don't make this case-insensitive.
As a user, I want to be able to set a relative time to specify the package versions to update. This commit extends the --uploaded-prior-to option to accept relative time spans in ISO format for duration in days (e.g., 'P3D'). The callback parses the given string into a time duration, and next computes and absolute timestamp. Then, we use the logic already present in uploaded_prior_to to process the absolute time. Unit tests are extended accordingly. Signed-off-by: Norbert Manthey <nmanthey@amazon.de>
Add towncrier news fragment for issue pypa#13674.
nmanthey
force-pushed
the
min_release_age
branch
from
7ae71ef to
cda89ed
Compare
|
I dropped the support for lower case letters, and adapted the unit tests accordingly. |
3 participants
As a user, I want to be able to set a relative time to specify the package versions to update. This feature is present in package managers of other repositories, e.g. in npm, as --min-release-age.
This commit introduces a new --min-release-age option to accept relative time spans (e.g., '7d', '168h', '10080m') as an alternative to --uploaded-prior-to.
The callback parses the given time stamp into an absolute time and next uses the logic already present via the uploaded_prior_to. Parsing supports d/days, h/hours, m/minutes, w/weeks as units. Unit tests are added accordingly, following the test pattern of --uploaded-prior-to.
Related issue #13674
Testing Done
Unit tests check the data parsing code similar to testing for the existing "--uploaded-prior-to" flag.
In addition, I tested the implementation in a local venv:
pip install certifipip install --min-release-age=10 certifipip install --min-release-age=365 certifiTesting Executed
Setup and install current pip
Check versions of certifi package
Install version from a year ago
Uninstall and install version from 10 days ago
Uninstall and install latest version