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

Generate release_date property in netkan #3059

Merged
merged 1 commit into from
Jun 30, 2020

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented May 21, 2020

Motivation

A longstanding request is to display when a mod was released, see #1155 and #2916.

This PR begins the process by capturing these values in current metadata where possible.

Changes

  • Now the schema and spec are updated to support an optional release_date property in ISO 8601 format
  • Now mods on SpaceDock, GitHub, and Jenkins will have a release_date property generated automatically based on API data
  • Other mods (there are 15 of them) will not have the property set, to indicate that we don't know the release date
  • CkanModule now has a release_date property that future changes can use to display the value to the user
  • The GitHubApi was attempting to parse the date properties returned by the GitHub API. However the JSON parsing library already parsed them into dates! The ToString call produced an intermediate format without timezone (rather than the original raw string as expected), so the TZ was lost. Now we use the date as provided by the parser.
  • The StagingTransformer will no longer stage modules where ksp_version_min is set to some prior version and ksp_version_max isn't set at all. This should help with the server IO load (staging is costly), as well as reduce the maintenance burden for mass metadata sweeps like this (lots of PRs to review).
  • The client now uses the release date to purge stale downloads just as Netkan has been doing since Invalidate stale cached files from GitHub in Netkan #2337 and Purge stale cache entries for SpaceDock #2859, which will ensure that users are installing the correct version of a replaced module

I plan to work on the UI side after this is merged (which will re-index almost all mods in a big sweep), so I can have plenty of test data.

Notes

  • We have been generating a release date value internally for some time now for purposes of cache purging (see Invalidate stale cached files from GitHub in Netkan #2337 and Purge stale cache entries for SpaceDock #2859), but they were populated into x_netkan_asset_updated and therefore stripped out by StripNetkanMetadataTransformer. Now that release_date is in the schema, we populate it instead and don't strip it.
  • Jenkins returns a timestamp representing milliseconds since 1970-01-01, and the default JSON UnixDateTimeConverter only supports seconds, so we have to roll our own converter.
  • RelationshipDescriptor and ResourcesDescriptor are split out into their own files to reduce clutter in CkanModule.cs
  • SpaceDock's API currently returns mod creation dates in local time, and Send UTC TZ for mod creation timestamps KSP-SpaceDock/SpaceDock#265 is fixing this to send UTC. Ideally we would wait until after that change is in production to merge this, to avoid multiple mass-update passes of SpaceDock hosted mods.

@HebaruSan HebaruSan added Enhancement New features or functionality Core (ckan.dll) Issues affecting the core part of CKAN Pull request Spec Issues affecting the spec Netkan Issues affecting the netkan data Schema Issues affecting the schema labels May 21, 2020
@HebaruSan HebaruSan requested a review from DasSkelett May 21, 2020 23:52
@DasSkelett
Copy link
Member

You are referencing the release date of "the mod" in the spec and schema, but isn't it more like the release date of "the mod version"/"the mod release"?
I think we should clarify this a bit.

@HebaruSan
Copy link
Member Author

Whoa, the schema says this:

"description" : "Version of the mod. Drop the leading v",

"Drop the leading v" is not something we've ever done. I'll go ahead and remove that now...

@HebaruSan
Copy link
Member Author

You are referencing the release date of "the mod" in the spec and schema, but isn't it more like the release date of "the mod version"/"the mod release"?
I think we should clarify this a bit.

Most of the existing text uses the same phrasing, but download in Spec.md says: "the described version of the mod"
Will switch to that...

@techman83
Copy link
Member

I'm very much in favour of this implementation, keeps our source of truth consistent and means we get our date/times from upstream.

@HebaruSan HebaruSan added the In progress We're still working on this label May 25, 2020
@HebaruSan
Copy link
Member Author

Planning to improve on #3031 so PRs like KSP-CKAN/CKAN-meta#1940 don't get submitted...

@HebaruSan HebaruSan removed the In progress We're still working on this label May 26, 2020
@HebaruSan
Copy link
Member Author

OK, I think that's done in the latest commit.

@HebaruSan
Copy link
Member Author

KSP-SpaceDock/SpaceDock#265 is in production! SpaceDock's timestamps have time zones!

Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

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

Let's go!

@HebaruSan HebaruSan merged commit ce9e36a into KSP-CKAN:master Jun 30, 2020
@HebaruSan HebaruSan deleted the feature/generate-release-date branch June 30, 2020 17:56
@HebaruSan HebaruSan mentioned this pull request Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality Netkan Issues affecting the netkan data Schema Issues affecting the schema Spec Issues affecting the spec
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants