Skip to content
This repository has been archived by the owner on Nov 20, 2024. It is now read-only.

[94] - added a default string for the package_id in case it’s not pre… #95

Merged

Conversation

kurtfoster
Copy link
Contributor

…sent in the response data.

@kurtfoster
Copy link
Contributor Author

@jeremy-doghouse , @seanhamlin , I've found another issue with the CKAN instance I'm working with. This should be a fairly minor one as well I believe.

#94

@stooit
Copy link

stooit commented May 8, 2017

Nice one, thanks @kurtsalsa. We didn't have a huge range of CKAN instances to test against -- out of interest what version?

@tobybellwood
Copy link

@kurtsalsa @jeremy-doghouse (or @SRowlands if you now know) can you provide an example of the url string you're handling that has a null PACKAGE_ID - I'm trying to think how this might occur?

From my reading of the code, am I correct that this stage is only needed to then get at the CKAN resource_id to then use in the datastore_search part of the module?

@jeremy-doghouse
Copy link
Contributor

@tobybellwood I think this might be when an alias is used rather than the /dataset/PACKAGE_ID/resource/RESOURCE_ID format? But @kurtsalsa can probably shed more light

@kurtfoster
Copy link
Contributor Author

tbh, we could probably just remove PACKAGE_ID all together and achieve the same result. The issue is only on versions of CKAN that don't use package id.

@kurtfoster
Copy link
Contributor Author

@tobybellwood tobybellwood changed the title [94] - added a default string for the pacakge_id in case it’s not pre… [94] - added a default string for the package_id in case it’s not pre… May 31, 2017
@tobybellwood
Copy link

tobybellwood commented May 31, 2017

The genesis of this problem is that in CKAN versions prior to 2.3, the API response to a package_show or resource_show did not contain the parent package_id within the resource response.

i.e. https://www.data.vic.gov.au/data/api/action/package_show?id=state-budget-2017-18-financial-management-data
and
https://www.data.vic.gov.au/data/api/action/resource_show?id=894f07bb-3fd9-44d1-9b12-93764b461442
(data.vic.gov.au is on 2.2.1?)

(versus http://data.gov.au/api/action/resource_show?id=59a967f8-6195-47bb-91a1-36c874a0da24)

I believe this patch enters a default string of 'package' for package_id to enable the process to continue.

I'm presuming that this is the only step that needs this default (or as @kurtsalsa pointed out - is it necessary at all) given the rest of the API calls seemingly don't depend on this gathered package_id - otherwise they would fail owing to it's fakeness.

If it's not needed anywhere else, is a better resolution to remove the need for it within this step?

@kurtfoster
Copy link
Contributor Author

@tobybellwood, FYI, DPC would really like to get the site onto SaaS. If this patch fixes the issue, can a new issue be created to investigate the relevance of the package_id long term. Currently, this is a blocker.

@stooit stooit merged commit 467ef1d into govCMS:develop Jun 8, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants