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

Store cache timestamps to status pynamodb #205

Merged

Conversation

HebaruSan
Copy link
Member

Motivation

Currently the ModStatus objects have properties last_checked and last_inflated, which are identical, which is redundant. This is not a coincidence; the code sets them to the exact same value every time. They also appear on the status page:

image

Problems

In KSP-CKAN/CKAN-meta@8006736, KerbalWeatherProject's release date was updated but its hashes were not. Subsequently attempting to install this mod resulted in hash mismatches:

CKAN.InvalidModuleFileKraken: KerbalWeatherProject v1.0.0: C:\Users\aequa\AppData\Local\Temp\tmp83D1.tmp has length 88819967, should be 88820268
   at CKAN.NetModuleCache.Store(CkanModule module, String path, String description, Boolean move)
   at CKAN.NetAsyncModulesDownloader.ModuleDownloadComplete(Uri url, String filename, Exception error)

So the author replaced the download, but the Inflator used the old download to re-index it.

Causes

There are two ways the Inflator might have inflated the old file:

Right now we cannot tell which it is, because we do not know the timestamp of the file in the Inflator's cache. We need that info to continue investigating and eventually find a fix. If the timestamp is close to the re-indexing request, then we blame SpaceDock for lying, otherwise we blame the Inflator for laziness.

Changes

Now last_checked is no longer set (but we won't purge the current values, just in case). last_inflated will still be set to their common value, so no information is lost.

Now a new last_downloaded property is set to the timestamp of the mod's download in the cache. To make this possible, the Indexer now mounts the cache directory, finds downloads in it, and stats them to get the timestamp. We carefully convert this to UTC to ensure that we can always track exactly what happened with a mod, minute by minute.

A future PR will update the status page to show the new timestamps.

@HebaruSan HebaruSan added Enhancement New feature or request Status It's about the inflation status (page or db) Indexer Receives inflated modules and adds them to CKAN-meta Environment It's about the dev/prod environment labels Jan 6, 2021
@HebaruSan HebaruSan requested a review from techman83 January 6, 2021 01:55
@HebaruSan

This comment has been minimized.

@HebaruSan HebaruSan force-pushed the feature/status-cache-timestamps branch from 0f11867 to dd4e788 Compare January 6, 2021 02:04
@techman83
Copy link
Member

Yep, 100% agree. The 'last_checked' field was an artifact of unnecessary future plans that never came to fruition. Realistically with all the improvements that have been made to the inflator, those ideas are no longer relevant. Love your work!

Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

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

Really only a minor implementation detail here, if effort is significant, filesystem timestamps will be sufficient.

netkan/netkan/indexer.py Show resolved Hide resolved
@techman83 techman83 merged commit 1ae93b2 into KSP-CKAN:master Jan 7, 2021
@HebaruSan HebaruSan deleted the feature/status-cache-timestamps branch January 7, 2021 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Environment It's about the dev/prod environment Indexer Receives inflated modules and adds them to CKAN-meta Status It's about the inflation status (page or db)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants