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

Support new Curse URLs with non-numeric IDs #2464

Merged
merged 1 commit into from
Jul 3, 2018

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Jun 28, 2018

Background

The format of Curse API URLs changed recently, but some old URLs are still supported; see the discussion in KSP-CKAN/NetKAN#6608 for details.

Old format:

New format:

Any old URL that had been accessed before March 2018 still works thanks to caching internal to the API, but any newly added mod must use the new format.

Problem

Netkan fails on mods added to curse after March 2018. See KSP-CKAN/NetKAN#6608 for an example; PhotonSailor can't be indexed without updating our Curse API logic.

(UPDATE: We switched that mod to SpaceDock instead, so the urgency is lower, but the problem would still affect future new mods hosted only on Curse.)

Changes

Now the id part of the #/ckan/curse/id kref string may contain either a number or a non-number. If it's a number, the old format URL is used for backwards compatibility with existing metadata. If it's not a number, then the new format is used.

In addition, the Netkan log message that prints the API URL being accessed is promoted from debug to info, so it can be seen when --verbose is enabled.

A test is added to exercise the new format.

@HebaruSan HebaruSan added Netkan Issues affecting the netkan data Tests Issues affecting the internal tests Network Issues affecting internet connections of CKAN labels Jun 28, 2018
@HebaruSan HebaruSan requested a review from Olympic1 June 29, 2018 01:55
Copy link
Member

@Olympic1 Olympic1 left a comment

Choose a reason for hiding this comment

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

LGTM, don't notice any problems

@politas politas merged commit 1790c6c into KSP-CKAN:master Jul 3, 2018
politas added a commit that referenced this pull request Jul 3, 2018
@HebaruSan HebaruSan deleted the fix/new-curse-urls branch July 3, 2018 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Netkan Issues affecting the netkan data Network Issues affecting internet connections of CKAN Tests Issues affecting the internal tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants