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

Get licenses from embedded schema, skip bad modules in deserialize #3526

Merged
merged 1 commit into from
Feb 3, 2022

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Jan 26, 2022

Problems

  • Every time we add a new license, the previous version of the CKAN client is unable to read modules that use it. This is supposed to be handled by spec_version, but the user can bypass this protection by updating the registry with a new client and then running an old client, and they have actually done this (see CKAN.BadMetadataKraken: The license MPL-2.0 is invalid #3331 and CKAN.BadMetadataKraken: The license MPL-2.0 is invalid #3410 and a reply on the forum thread).
  • We currently maintain the list of valid licenses in three places, which is annoying:
    • CKAN/Core/Types/License.cs
    • CKAN/CKAN.schema
    • NetKAN-Infra/netkan/netkan/mirrorer.py

Changes

  • Now if any CkanModule in the available modules list throws an exception during deserialization, including but not limited to an unsupported license, we log a warning and omit that module from its AvailableModule collection. This allows the user to keep using the client, albeit without access to such modules.
    • This is done with a new JsonConverter that adds each version/module pair to a SortedDictionary one at a time and returns it containing whatever values didn't throw.
  • Now CKAN.schema is an embedded resource in Core, and Licenses.cs retrieves its license data from there.
    • A CKANSchema class holds a schema object generated by extracting the embedded resource and then parsing it with NJsonSchema
    • Netkan's schema validator now uses CKANSchema.schema instead of its own embedded resource
    • A .definitions.redistributable_license enum is added to the schema to document which licenses allow us to redistribute.

@HebaruSan HebaruSan added Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality Netkan Issues affecting the netkan data Pull request Registry Issues affecting the registry Schema Issues affecting the schema labels Jan 26, 2022
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.

Nice deduplication

@HebaruSan HebaruSan merged commit 46e9890 into KSP-CKAN:master Feb 3, 2022
@HebaruSan HebaruSan deleted the feature/embed-schema branch February 3, 2022 16:44
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 Registry Issues affecting the registry Schema Issues affecting the schema
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants