Add support for deferred updates / minimum release age#10824
Open
radoering wants to merge 3 commits intopython-poetry:mainfrom
Open
Add support for deferred updates / minimum release age#10824radoering wants to merge 3 commits intopython-poetry:mainfrom
radoering wants to merge 3 commits intopython-poetry:mainfrom
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
HTTPRepository._find_packages, the debug log for ignored versions currently interpolates the fullversionslist rather than justfiltered_out, which makes the message misleading; consider logging only the actually filtered versions (e.g.,filtered_outorself._age_filtered_versions[name]).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `HTTPRepository._find_packages`, the debug log for ignored versions currently interpolates the full `versions` list rather than just `filtered_out`, which makes the message misleading; consider logging only the actually filtered versions (e.g., `filtered_out` or `self._age_filtered_versions[name]`).
## Individual Comments
### Comment 1
<location path="src/poetry/repositories/http_repository.py" line_range="200-202" />
<code_context>
+ else:
+ accepted.append((version, yanked))
+ if filtered_out:
+ self._age_filtered_versions[name] = filtered_out
+ self._log(
+ f"Ignoring {name} version(s) due to "
+ f"solver.min-release-age={self._min_release_age}: {versions}",
+ level="debug",
</code_context>
<issue_to_address>
**issue:** The debug message logs all versions, not just the filtered-out ones, which can be misleading.
Here `versions` still includes both accepted and filtered entries. Since you’ve already collected excluded versions in `filtered_out`, the log should use `filtered_out` (or clearly distinguish accepted vs. ignored) so the message accurately reflects which versions are actually being ignored.
</issue_to_address>
### Comment 2
<location path="src/poetry/repositories/http_repository.py" line_range="100" />
<code_context>
+ canonicalize_name(n)
+ for n in (config.get("solver.min-release-age-exclude") or [])
+ }
+ self._age_filtered_versions: dict[NormalizedName, set[Version]] = {}
# We are tracking if a domain supports range requests or not to avoid
# unnecessary requests.
</code_context>
<issue_to_address>
**suggestion (bug_risk):** _age_filtered_versions is never reset, so logs may accumulate stale entries across multiple solves.
Because `_age_filtered_versions` is stored on the repository instance and only ever appended to, entries from earlier solves will still be present when `log_age_filtered_versions()` runs in later solves. If this log is meant to reflect only the current solve, clear `_age_filtered_versions` at the start of each solve (or resolution round), or store it in per-solve state instead of on the repository instance.
</issue_to_address>
### Comment 3
<location path="tests/repositories/test_http_repository.py" line_range="161-170" />
<code_context>
+ ],
+ True,
+ ),
+ ( # some files old enough, some files not
+ [
+ Link("https://foo.com/pkg-1.0.tar.gz"),
+ Link(
+ "https://foo.com/pkg-1.0-py3-none-any.whl",
+ upload_time="2017-08-15T00:00:00Z",
+ ),
+ ],
+ True,
+ ),
+ ( # all files not old enough
</code_context>
<issue_to_address>
**suggestion (testing):** Parametrized cases for `_is_version_too_recent` contain a duplicated scenario; likely missing the intended "mixed old/new" upload-time case
In `test_is_version_too_recent`, the third and fourth parametrized cases use the same `links` (one `Link` without `upload_time`, one with `upload_time="2017-08-15T00:00:00Z"`) but describe different scenarios. As written, the "some files old enough, some files not" case never exercises a mix of pre‑ and post‑cutoff upload times. Please either adjust that case so one link is before `2017-08-10` and the other after, or remove it if the extra scenario isn’t needed.
```suggestion
( # some files old enough, some files not
[
Link(
"https://foo.com/pkg-1.0.tar.gz",
upload_time="2017-08-01T00:00:00Z",
),
Link(
"https://foo.com/pkg-1.0-py3-none-any.whl",
upload_time="2017-08-15T00:00:00Z",
),
],
True,
),
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
radoering
force-pushed
the
min-release-age
branch
from
45598d2 to
f8a008f
Compare
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The matching logic for
solver.min-release-age-exclude-sourceonly compares raw strings againstnameandurl, so you may want to normalise URLs (e.g. trailing slashes, case) and/or source names to avoid surprising mismatches between config and repository definitions. - In
_find_packages, the debug log for filtered versions uses the rawset[Version]representation, whereaslog_age_filtered_versionsformats them nicely; consider reusing the same formatting helper there so logs are consistent and easier to read.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The matching logic for `solver.min-release-age-exclude-source` only compares raw strings against `name` and `url`, so you may want to normalise URLs (e.g. trailing slashes, case) and/or source names to avoid surprising mismatches between config and repository definitions.
- In `_find_packages`, the debug log for filtered versions uses the raw `set[Version]` representation, whereas `log_age_filtered_versions` formats them nicely; consider reusing the same formatting helper there so logs are consistent and easier to read.
## Individual Comments
### Comment 1
<location path="tests/repositories/test_http_repository.py" line_range="99-108" />
<code_context>
+@pytest.mark.parametrize(
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test case where upload_time is exactly equal to the cutoff to pin down the boundary condition for `_is_version_too_recent`.
The existing `test_is_version_too_recent` cases cover only `< cutoff` and `> cutoff`. Because `_is_version_too_recent` uses a strict `>` comparison, an upload exactly at `_min_release_age_cutoff` should return `False`. Please add a parameterized case with `upload_time == datetime(2017, 8, 10, tzinfo=timezone.utc)` to codify this boundary behavior and guard against future changes to `>=`.
Suggested implementation:
```python
@pytest.mark.parametrize(
("upload_time", "expected"),
[
(datetime(2017, 8, 9, tzinfo=timezone.utc), False), # before cutoff
(datetime(2017, 8, 10, tzinfo=timezone.utc), False), # exactly at cutoff
(datetime(2017, 8, 11, tzinfo=timezone.utc), True), # after cutoff
],
)
def test_is_version_too_recent(
```
If the existing `test_is_version_too_recent` uses different parameter names, variable names, or cutoff dates, adjust the three tuple entries accordingly so that:
1. One case is strictly before `_min_release_age_cutoff` and expects `False`.
2. The new case is exactly equal to `_min_release_age_cutoff` and expects `False`.
3. One case is strictly after `_min_release_age_cutoff` and expects `True`.
Also ensure that `datetime` and `timezone` are imported from the `datetime` module at the top of `tests/repositories/test_http_repository.py` (they likely already are).
</issue_to_address>
### Comment 2
<location path="tests/config/test_config.py" line_range="47-48" />
<code_context>
("virtualenvs.create", True),
("requests.max-retries", 0),
+ ("solver.min-release-age", 0),
],
)
-def test_config_get_default_value(config: Config, name: str, value: bool) -> None:
+def test_config_get_default_value(config: Config, name: str, value: bool | int) -> None:
assert config.get(name) is value
</code_context>
<issue_to_address>
**suggestion (testing):** Add dedicated tests for `str_list_normalizer` and the config mapping of the new solver-related options.
The new integer default is covered via `("solver.min-release-age", 0)`, but the list-based options and their normalizer aren’t. Please add small tests that:
- Assert `_get_normalizer("solver.min-release-age-exclude-source") is str_list_normalizer`.
- Assert `str_list_normalizer("foo, bar , baz") == ["foo", "bar", "baz"]` and that empty segments (e.g. `", ,"`) are dropped.
This will make the normalization behavior explicit and guard against regressions beyond the CLI tests.
Suggested implementation:
```python
def test_config_get_default_value(config: Config, name: str, value: bool | int) -> None:
assert config.get(name) is value
def test_solver_min_release_age_exclude_source_uses_str_list_normalizer() -> None:
assert _get_normalizer("solver.min-release-age-exclude-source") is str_list_normalizer
@pytest.mark.parametrize(
("value", "expected"),
[
("foo, bar , baz", ["foo", "bar", "baz"]),
(", ,", []),
],
)
def test_str_list_normalizer_trims_and_ignores_empty_segments(value: str, expected: list[str]) -> None:
assert str_list_normalizer(value) == expected
```
To make these tests work, ensure the following at the top of `tests/config/test_config.py`:
1. `pytest` is imported (if not already present):
`import pytest`
2. `_get_normalizer` and `str_list_normalizer` are imported from the module where they are defined, likely:
`from poetry.config.config import Config, _get_normalizer, str_list_normalizer`
adjusting the import path if your project structure differs.
</issue_to_address>
### Comment 3
<location path="docs/configuration.md" line_range="426-434" />
<code_context>
+
+*Introduced in 2.4.0*
+
+A comma-separated list of package names that should be excluded from the
+[`solver.min-release-age`](#solvermin-release-age) filter.
+Versions of these packages will always be considered by the solver,
+regardless of their upload age.
+
+```bash
+poetry config solver.min-release-age-exclude "my-package,other-package"
+```
+
</code_context>
<issue_to_address>
**suggestion:** Consider clarifying how the comma-separated list is parsed (whitespace, case-sensitivity, and name normalization).
It would help to document exactly how this list is interpreted: whether spaces around commas are allowed (e.g. `"my-package, other-package"`), whether matching is case-sensitive, and whether names are normalized like in `pyproject.toml` (hyphens vs underscores). This can help avoid misconfigurations.
```suggestion
### `solver.min-release-age-exclude`
**Type**: `string`
**Default**: *not set*
**Environment Variable**: `POETRY_SOLVER_MIN_RELEASE_AGE_EXCLUDE`
*Introduced in 2.4.0*
A comma-separated list of package names that should be excluded from the
[`solver.min-release-age`](#solvermin-release-age) filter. Versions of these
packages will always be considered by the solver, regardless of their upload age.
Names in this list:
- are split on commas, with surrounding whitespace ignored
(for example, `poetry config solver.min-release-age-exclude "foo, bar"` is valid),
- are matched case-insensitively, and
- are normalized in the same way as package names in `pyproject.toml`
(for example, `my_package` and `my-package` are treated as the same name).
```bash
poetry config solver.min-release-age-exclude "my-package,other-package"
```
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This was referenced Apr 5, 2026
Contributor
|
Havent looked too closely at this, does it bring any reason to revive #9170 (stop using the JSON API)? Feels like it might, but perhaps not. |
Member
Author
I do not think that it changes anything since the JSON API provides the relevant information. |
radoering
force-pushed
the
min-release-age
branch
from
f8a008f to
1b142ec
Compare
Member
Author
Good point. I changed the name comparison to use lower case because we consider source names case-insensitive.
Actually, output is worse than I thought: |
radoering
force-pushed
the
min-release-age
branch
from
1b142ec to
9f57013
Compare
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
HTTPRepository._is_name_excluded_from_min_release_ageand_is_url_excluded_from_min_release_age, you rebuild normalized sets on every call; consider normalizing and storing the lowercased names and stripped URLs once in__init__to avoid repeated allocations and improve clarity. - The base
Repository.log_age_filtered_versionscurrently justpasses; if it is expected to be invoked viaRepositoryPool.log_age_filtered_versions, it might be clearer to add a docstring or make it explicitly a no-op hook so that future subclasses know they can override it to participate in the logging.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `HTTPRepository._is_name_excluded_from_min_release_age` and `_is_url_excluded_from_min_release_age`, you rebuild normalized sets on every call; consider normalizing and storing the lowercased names and stripped URLs once in `__init__` to avoid repeated allocations and improve clarity.
- The base `Repository.log_age_filtered_versions` currently just `pass`es; if it is expected to be invoked via `RepositoryPool.log_age_filtered_versions`, it might be clearer to add a docstring or make it explicitly a no-op hook so that future subclasses know they can override it to participate in the logging.
## Individual Comments
### Comment 1
<location path="tests/repositories/test_repository_pool.py" line_range="293-302" />
<code_context>
assert repo2.search("demo") == pool.search("demo") == [demo_package]
+
+
+@pytest.mark.parametrize(("level", "reset"), [("warning", True), ("info", False)])
+def test_log_age_filtered_versions_includes_explicit_repositories(
+ mocker: MockerFixture, level: str, reset: bool
+) -> None:
+ primary = Repository("primary")
+ explicit = Repository("explicit")
+
+ primary_spy = mocker.spy(primary, "log_age_filtered_versions")
+ explicit_spy = mocker.spy(explicit, "log_age_filtered_versions")
+
+ pool = RepositoryPool([primary])
+ pool.add_repository(explicit, priority=Priority.EXPLICIT)
+
+ pool.log_age_filtered_versions(level=level, reset=reset)
+
+ primary_spy.assert_called_once_with(level=level, reset=reset)
+ explicit_spy.assert_called_once_with(level=level, reset=reset)
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding solver-level tests to verify `log_age_filtered_versions` is called on both success and failure paths
This test confirms `RepositoryPool.log_age_filtered_versions` fans out correctly, but `Solver.solve` now also calls it with `level="warning", reset=True` on `SolverProblemError` and `level="info"` on success. Please add solver-level tests (e.g. in `tests/puzzle/test_solver.py`) that spy on `RepositoryPool.log_age_filtered_versions` and assert these two call patterns on failure vs success to lock in this behaviour and avoid regressions.
Suggested implementation:
```python
import logging
```
```python
def test_solver_logs_age_filtered_versions_on_failure(
mocker: MockerFixture,
repository_pool: RepositoryPool,
solver: Solver,
) -> None:
"""
When the solver fails with a SolverProblemError, it should call
RepositoryPool.log_age_filtered_versions(level="warning", reset=True).
"""
log_age_filtered_versions_spy = mocker.spy(
repository_pool, "log_age_filtered_versions"
)
# Force a SolverProblemError from solve()
mock_solve = mocker.patch.object(
solver, "_solve", side_effect=SolverProblemError("test failure")
)
with pytest.raises(SolverProblemError):
solver.solve()
mock_solve.assert_called_once()
log_age_filtered_versions_spy.assert_called_once_with(level="warning", reset=True)
def test_solver_logs_age_filtered_versions_on_success(
mocker: MockerFixture,
repository_pool: RepositoryPool,
solver: Solver,
) -> None:
"""
When the solver succeeds, it should call
RepositoryPool.log_age_filtered_versions(level="info", reset=False).
"""
log_age_filtered_versions_spy = mocker.spy(
repository_pool, "log_age_filtered_versions"
)
# Use a trivial, successful _solve implementation
mock_result = mocker.Mock()
mocker.patch.object(solver, "_solve", return_value=mock_result)
result = solver.solve()
assert result is mock_result
log_age_filtered_versions_spy.assert_called_once_with(level="info", reset=False)
```
1. Ensure the following imports and fixtures are available in `tests/puzzle/test_solver.py` (or adjust the code above to match existing ones):
- `from poetry.puzzle.solver import Solver, SolverProblemError`
- `from poetry.repositories import RepositoryPool`
- `from _pytest.monkeypatch import MockerFixture` (or the correct `mocker` fixture import for your test setup).
- `import pytest`
2. If `repository_pool` and `solver` fixtures do not already exist in `tests/puzzle/test_solver.py`, create them (or adapt the tests to use the existing construction helpers), ensuring:
- `solver` is constructed with the `repository_pool` instance used in the spy, so that `solver.solve()` internally calls `repository_pool.log_age_filtered_versions(...)`.
- `_solve` is the internal method invoked by `Solver.solve` in your codebase; if the internal method name differs, update the `mocker.patch.object(solver, "_solve", ...)` calls accordingly.
3. If your implementation of `Solver.solve` passes different `reset` arguments (e.g., always `True` on both paths or uses configuration), adjust the asserted arguments in `assert_called_once_with(...)` to match the intended behaviour.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
radoering
force-pushed
the
min-release-age
branch
from
9f57013 to
64e8758
Compare
Member
Author
The methods are only called once in
I added a docstring. |
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
HTTPRepository.__init__,_is_name_excluded_from_min_release_ageand_is_url_excluded_from_min_release_agerebuild lowercased/stripped sets every time they are called; consider normalizing the exclude list once up front (e.g. precomputed lowercased names and rstrip('/') URLs) to avoid repeated work. - In
_find_packages,self._age_filtered_versions[name] = filtered_outoverwrites any previously recorded filtered versions for the same package name; if a repository is queried multiple times with different constraints, you may want to accumulate (|=) the versions instead of replacing them so the subsequent logging shows the complete set filtered during the run.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `HTTPRepository.__init__`, `_is_name_excluded_from_min_release_age` and `_is_url_excluded_from_min_release_age` rebuild lowercased/stripped sets every time they are called; consider normalizing the exclude list once up front (e.g. precomputed lowercased names and rstrip('/') URLs) to avoid repeated work.
- In `_find_packages`, `self._age_filtered_versions[name] = filtered_out` overwrites any previously recorded filtered versions for the same package name; if a repository is queried multiple times with different constraints, you may want to accumulate (`|=`) the versions instead of replacing them so the subsequent logging shows the complete set filtered during the run.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
radoering
force-pushed
the
min-release-age
branch
from
64e8758 to
5cb23ae
Compare
Member
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
_is_url_excluded_from_min_release_age, you only strip trailing slashes but otherwise compare URLs as-is; consider normalizing scheme/host case and perhaps resolving minor differences (e.g.httpvshttps, trailing/simple/) so that configuration forsolver.min-release-age-exclude-sourcebehaves more predictably across equivalent URLs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_is_url_excluded_from_min_release_age`, you only strip trailing slashes but otherwise compare URLs as-is; consider normalizing scheme/host case and perhaps resolving minor differences (e.g. `http` vs `https`, trailing `/simple/`) so that configuration for `solver.min-release-age-exclude-source` behaves more predictably across equivalent URLs.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
Out of scope for now. Trailing slashes are only removed because we have already done this ourselves in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Check List
Resolves: #10646
Closes: #10763, #10813
Adds three new config options (see
docs/configuration.mdfor details) - one per commit:solver.min-release-agesolver.min-release-age-excludesolver.min-release-age-exclude-sourceDesign decisions:
Location
I prefer a config option over a
pyproject.tomlsetting. Both have their merits. See https://github.com/orgs/python-poetry/discussions/10555#discussioncomment-14461277I put it under
solverbecause the filtering is done during dependency resolution. At install time, the locked versions are installed - no additional check for upload time.Naming
#10646 (comment) lists different names used by several tools. I decided for the most common one, but abbreviated "minimum" with "min" for consistency - we already have "max-workers" and "max-retries".
Values
I decided to keep it simple for now: Just plain integers, which are "days". I think "days" makes the most sense.
We can still allow ISO 8601 durations (e.g. "PT24H", "P7D", "P30D") or "friendly" durations (e.g. "24 hours", "1 week", "30 days") later because they should not conflict with plain integers.
Overrides for packages
Some tools just allow to exclude packages from the filtering, others allow to set individual min-release-age values per package. Same as before: I want to keep it simple for now so that I decided for
min-release-age-excludebeing just a list of package names since we already have other settings that are a list of package names. We can still introduce amin-release-age-package(or similar) later if there is enough demand.Overrides for sources
This is a setting I did not see in other tools (maybe I have just missed it?), but I implemented it anyway because I think it is quite useful and it was simple enough to implement.
min-release-age-exclude-sourceallows to list sources you want to exclude from the filtering. I believe a common use case might be that you only want to do deferred updates for public packages (from PyPI for example) but not for your internal packages from your private index.