Repository navigation
馃悰 fix(seed): guard every embed update log read, not just one - #3383
Merged
gaborbernat merged 4 commits intoOct 8, 2026
Merged
Conversation
pypa#3376 made UpdateLog.from_app_data drop an update log that holds JSON of the wrong shape, but periodic_update() only reaches that guard after handle_auto_update() has already parsed the same file with the raw UpdateLog.from_dict(). Periodic update is on by default, so the guard only helped runs with --no-periodic-update. Everyone else still hit the TypeError: the app-data seeder logged it as "fail" with a traceback, then retried with download=True and fetched pip from PyPI even though the user never asked for a download. With no network (PIP_NO_INDEX=1 reproduces it) environment creation failed with "seed failed due to failing to download wheels pip". add_wheel_to_update_log() and the background do_update() had the same unguarded read. All three now go through from_app_data, so a malformed log is dropped with one warning wherever it is first read, and the reset log schedules a fresh periodic update as a never-run log would. The malformed-log cases are shared across the four entry points; each new test fails when its call site is put back to from_dict.
for more information, see https://pre-commit.ci
Write each malformed log through one parametrized fixture instead of repeating the setup in four tests, inline the update log stores that the guarded readers now touch once, and state the changelog fix in user terms.
gaborbernat
enabled auto-merge (squash)
October 8, 2026 05:53
gaborbernat
disabled auto-merge
October 8, 2026 14:47
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
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.

#3376 made
UpdateLog.from_app_datadrop an embed update log that holds JSON of the wrong shape. Three other readers inperiodic_update.pystill parse the file with the rawUpdateLog.from_dict(embed_update_log.read()), and one of them runs first on the default path, so the guard only helps runs with--no-periodic-update.With periodic update on (the default),
handle_auto_updatehits the bad log beforefrom_app_dataever sees it. The app-data seeder logs the TypeError asfailwith a traceback, then retries withdownload=Trueand pulls pip from PyPI even though nobody asked for a download. Offline it fails outright:#3376's own test passeddo_periodic_update=False, which is why it never reached this.add_wheel_to_update_logand the background_run_do_updatehave the same unguarded read. All three now go throughfrom_app_data, so a malformed log gets dropped with one warning wherever it's first read, and the reset log schedules a fresh periodic update the way a never-run log does.The malformed-log cases are shared across the four entry points as
_MALFORMED_LOGS. Each new test fails when its own call site is put back tofrom_dict. The full suite, ruff, and ty (3.9 and 3.14) pass.Something I noticed but left alone:
test_periodic_update_skipandtest_periodic_update_triggerpassos.environ, Truepositionally into(do_periodic_update, env), so the arguments are swapped. They only pass becauseos.environis truthy. Happy to send that as a separate fix if you want it.