Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use content hash vs. freshness in publish registry #498

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jablko
Copy link
Contributor

@jablko jablko commented Jul 9, 2022

Same as the calculate versions and retag scripts, when gathering npm metadata, always revalidate if the content hashes differ (fresh or not) and never revalidate if they match (stale or not). This will use the cache for types that haven't changed since the last run, hitting the origin (npm registry) less often. If we have the content hash from the DT repo and the cached metadata, we might as well use them.

@jablko jablko force-pushed the patch-62 branch 7 times, most recently from 37617ac to 33bb063 Compare July 14, 2022 17:35
Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compared to the retag script, how many accesses to npm do you expect this will save? I didn't think that many were happening on an average run.

The fact that this code is in the retag script already is a good argument for copying it here -- but I'd like a good way to test that this is working. I guess deploying and watching a publish or two is one way, but is there some pattern of usage that would empty a lot of the cache and make it obvious that it is correctly deciding when to re-download from npm? I want to be confident of this change since it could cause subtle problems if it's wrong.

@@ -1,6 +1,8 @@
import assert = require("assert");
import process from "process";
import { emptyDir } from "fs-extra";
// @ts-expect-error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the error?

pickManifest(offline, `~${typing.major}.${typing.minor}`).typesPublisherContentHash === typing.contentHash
)
return offline;
// @ts-expect-error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants