feat(crate): Support releaseTimestamp
#31467
Conversation
| private static releaseTimestampSchema = z | ||
| .object({ | ||
| version: z.object({ | ||
| created_at: z.string(), | ||
| }), | ||
| }) | ||
| .transform(({ version: { created_at } }) => created_at); |
There was a problem hiding this comment.
Don't we normally put these in a schema.ts?
There was a problem hiding this comment.
Yes, didn't feel like it creating the new file, will do this later
| { registryUrl, packageName }: PostprocessReleaseConfig, | ||
| { version }: Release, | ||
| ) => `postprocessRelease:${registryUrl}:${packageName}:${version}`, | ||
| ttlMinutes: 24 * 60, |
There was a problem hiding this comment.
Are releases immutable on crates.io? If so then we should think about the maximum feasible cache time for this - perhaps a week?
There was a problem hiding this comment.
We should also make sure we've considered the scenario where we get a temporary error (including 429) and don't cache that for long
There was a problem hiding this comment.
It could be yanked, and also there is updated_at field, so let's not consider it immutable.
We should also make sure we've considered the scenario where we get a temporary error (including 429) and don't cache that for long
With current cache decorator implementation, it's hard to set the TTL a posteriori. If the error was thrown, we could either not cache it, or we could catch and return the cacheable null (without distinguishing the reason).
| const { body: releaseTimestamp } = await this.http.getJson( | ||
| url, | ||
| CrateDatasource.releaseTimestampSchema, | ||
| ); |
There was a problem hiding this comment.
Does this need a try/catch for safety?
There was a problem hiding this comment.
I think we're okay with standard error handling, which just returns the release object intact
Changes
Context
Documentation (please check one with an [x])
How I've tested my work (please select one)
I have verified these changes via: