Conversation
MikeMcQuaid
left a comment
MikeMcQuaid
left a comment
There was a problem hiding this comment.
This approach looks very good and promising @samford, thanks for this and nice work on it. This is pretty much exactly what I had in mind, good job reading my 🧠. Let me know if I can provide any help or context here!
|
Just noting that for PyPI formulae, it looks like the change to resource update does implicitly apply the cooldown to main formula, e.g. from last autobump run: ❯ python3 -m pip install -q --disable-pip-version-check --dry-run --ignore-installed --uploaded-prior-to=2026-04-04T17:43:48Z --report=/dev/stdout ty==0.0.29
ERROR: Ignored the following yanked versions: 0.0.1
ERROR: Could not find a version that satisfies the requirement ty==0.0.29 (from versions: 0.0.0a1, 0.0.0a4, 0.0.0a5, 0.0.0a6, 0.0.0a7, 0.0.0a8, 0.0.1a1, 0.0.1a2, 0.0.1a3, 0.0.1a4, 0.0.1a5, 0.0.1a6, 0.0.1a7, 0.0.1a8, 0.0.1a9, 0.0.1a10, 0.0.1a11, 0.0.1a12, 0.0.1a13, 0.0.1a14, 0.0.1a15, 0.0.1a16, 0.0.1a17, 0.0.1a18, 0.0.1a19, 0.0.1a20, 0.0.1a21, 0.0.1a22, 0.0.1a23, 0.0.1a24, 0.0.1a25, 0.0.1a26, 0.0.1a27, 0.0.1a28, 0.0.1a29, 0.0.1a30, 0.0.1a31, 0.0.1a32, 0.0.1a33, 0.0.1a34, 0.0.1a35, 0.0.2, 0.0.3, 0.0.4, 0.0.5, 0.0.6, 0.0.7, 0.0.8, 0.0.9, 0.0.10, 0.0.11, 0.0.12, 0.0.13, 0.0.14, 0.0.15, 0.0.16, 0.0.17, 0.0.18, 0.0.19, 0.0.20, 0.0.21, 0.0.22, 0.0.23, 0.0.24, 0.0.25, 0.0.26, 0.0.27, 0.0.28)Though still need to make For npm, it would be nice to get something in earlier and improve it in follow ups. Right now we need to manually delay CI runs by a day:
Assuming complexity of re-implementing version resolution from
If complexity grows too much, then may consider delegating to |
Is the time due to some special parsing we are doing? EDIT: or perhaps our user-agent is throttled? Just a naive decode/encode in hash seems quick: ❯ time (brew ruby -e 'p JSON.parse(`curl -s --compressed https://registry.npmjs.org/wrangler`).to_json.size')
29254916
( brew ruby -e ; ) 0.64s user 0.23s system 62% cpu 1.387 total |
This implements Mike's idea to add a cooldown to `brew bump` for npm and PyPI packages, in light of ongoing security incidents in npm in particular. The `version_with_cooldown` method checks upstream sources for version and release date information and identifies the highest version that was released before the cooldown interval. This works based on very limited manual testing but there are some caveats: * The keys in the `releases` field in the PyPI JSON are sorted using string comparison (e.g., 1.2.30 is latest but 1.2.4, 1.2.5, etc. are after it), so this sorts using `Version` comparison before reverse iterating to find the highest suitable version. This works when the package uses a typical version scheme but I'm not sure if this will work as expected for all packages, so I'll have to do more testing. * npm packages can contain a variety of different version streams (e.g., dev, legacy, stable) with releases interleaved. As with the PyPI approach, this sorts using `Version` comparison before reverse iterating. Depending on how upstream handles versions, it may be possible for this approach to pick an unstable version, so this is something that I may need to rework. * npm packages with thousands of releases will have a JSON response that's several MB and this takes a notable amount of time to download and parse (e.g., `wrangler` is ~30 MB and takes ~30 seconds). This comes into play whenever livecheck surfaces a new version, so ideally we would cache the response etag and JSON data between bump runs to allow us to use `If-None-Match` in requests and avoid unnecessary downloads (like `npm` and `pip`). This is especially an issue for packages with an aggressive release cadence, where there may always be a new version available due to the cooldown interval. However, those may be good candidates to use `throttle` instead. This still needs tests but it works as a proof of concept at this stage.
I was thinking the same thing, so that makes sense to me 👍
This is the harder part, as For the moment, I've added some very naive logic to skip any version with a hyphen if the current version doesn't include a hyphen. This should skip prerelease versions for formulae using a stable version but I imagine there will probably be outliers at some point. I haven't done any comprehensive testing across related formulae, so the question remains whether this will be good enough for the time being and we can iterate after merging. Otherwise, if you have a specific idea of how to handle this by calling out to
I narrowed the slowness down to the I'm going to push the aforementioned changes in a moment but I'm aiming to add some tests tomorrow (EST). |
This adds some additional logic to npm cooldown handling to ensure that we're only checking versions that are between the current and latest versions. This also skips versions that include a hyphen if the current version doesn't include a hyphen, as a very naive way of skipping prerelease versions if the current version is stable. I've only done basic testing of this and there may be outliers but it may handle simple scenarios, at least.
samford
force-pushed
the
bump-add-cooldown-for-npm-and-pypi
branch
from
27d5823 to
fae891d
Compare
I think so 👍🏻 |
@samford Personally I think this could land as-is and tests be added in a follow-up, given we've got (admittedly broken by me!) homebrew/core |
3 participants
brew lgtm(style, typechecking and tests) with your changes locally?This implements Mike's idea to add a cooldown to
brew bumpfor npm and PyPI packages, in light of ongoing security incidents in npm in particular. Theversion_with_cooldownmethod checks upstream sources for version and release date information and identifies the highest version that was released before the cooldown interval.This works based on very limited manual testing but there are some caveats:
releasesfield in the PyPI JSON are sorted using string comparison (e.g., 1.2.30 is latest but 1.2.4, 1.2.5, etc. are after it), so this sorts usingVersioncomparison before reverse iterating to find the highest suitable version. This works when the package uses a typical version scheme but I'm not sure if this will work as expected for all packages, so I'll have to do more testing.Versioncomparison before reverse iterating. Depending on how upstream handles versions, it may be possible for this approach to pick an unstable version, so this is something that I may need to rework.wrangleris ~30 MB and takes ~30 seconds). This comes into play whenever livecheck surfaces a new version, so ideally we would cache the response etag and JSON data between bump runs to allow us to useIf-None-Matchin requests and avoid unnecessary downloads (likenpmandpip). This is especially an issue for packages with an aggressive release cadence, where there may always be a new version available due to the cooldown interval. However, those may be good candidates to usethrottleinstead.This still needs tests but it works as a proof of concept at this stage. I've opened this as a draft PR, so we can discuss and collaborate on an implementation. Feel free to build on this if it's a viable foundation. I'm not married to this implementation and I was primarily interested in identifying issues that we may face and ensuring that we don't need more information from livecheck. If there are better approaches, we should consider those.