[ckan-dev] Confused on response['success'] and HTTP status codes with empty results

Tue Apr 2 16:24:20 UTC 2013

I agree that there is scope for both Sean's general purpose function,
and Daniel's more pythonic functions. I did exactly this in ckanclient
and it works fine:

    def action(self, action_name, **kwargs):
         ...

    def package_show(self, package_id):
        return self.action('package_show', id=package_id)

I only did a few, so perhaps others will benefit from your
comprehensive list of calls.

However I would advise against anything but a thin client. It should
abstract away the HTTP-isms only and not try to 'fix' CKAN APIs. In
the example for resource_search, you:
* repeat the documentation - breaks the DRY principal
* have a sanitize_params method which suggests the params need
'sanitizing'. This appears to remove blank keys and other magic which
appears to be your own preferences.

If there are niggles with the API then I'd suggest you improve them at
source, rather than write custom 'fixes' in one particular client.

Dave

On 2 April 2013 10:31, Daniel Graziotin <dgraziotin at task3.cc> wrote:
>> -----Original Message-----
>> From: ckan-dev-bounces at lists.okfn.org [mailto:ckan-dev-
>> bounces at lists.okfn.org] On Behalf Of Sean Hammond
>> Sent: lunedì 1 aprile 2013 22.48
>> To: ckan-dev at lists.okfn.org
>> Subject: Re: [ckan-dev] Confused on response['success'] and HTTP status
>> codes with empty results
>>
>> My worry about this is that it looks like you've implemented a function in
>> libckan for each function in the CKAN API.
>
> Yes, it is how I am implementing it.
>
>> This won't work because:
>>
>> - As new versions of CKAN are released with new API functions, those API
>>   functions will have to be added to libckan as well
>
> This would be a minor issue, as client libraries are usually synchronized with new API releases.
> I would take care of updating libckan. Hopefully somebody else will join the project at some point.
> After some testing/tweaking of the new Action APIs there will be some stability there, right? :-)
>
>> - Different sites run different versions of CKAN, with different API
>>   functions available, and we probably want the same version of
>>   libckan to be able to to interact with different sites
>>
>> - Some sites may have CKAN extensions installed that add custom action
>>   functions to the API, and these functions would not be usable with
>>   libckan, unless those sites also provide a custom version of
>>   libckan.
>>
>> All in all it seems simpler and better if the user supplies the name of the
>> action function they want to call as a string, like:
>>
>>     ckanclient.post_to_ckan_api('package_create', name='foobar')
>>
>
> Everything you write makes sense. I am not that much convinced with the hacker-style ckanclient.post_to_ckan_api() approach.
> It would not help that much developers, especially newcomers and those not part of ckan-core dev team.
> I think there is a solution. Let me first argue on the advantages of the current libckan approach.
>
> CKAN is becoming huge. It is difficult to follow the development cycle if you are not a core developer.
> Although there is a lot of documentation, we all know how much challenging is to keep it consistent and updated, no matter the quality and the commitment of developers.
>
> My target in writing libckan is to hide CKAN internal mechanisms to developers and let them just study/use the library.
> There are several advantages in defining the functions one by one.
> See the following snippet taken from libckan:
> ---
> def resource_search(client=client.Client(), query='', fields='', order_by='', offset='', limit=''):
>     """
>     Searches for resources satisfying a given search criteria.
>     [...]
>     :param client: the CKAN Client.
>         Default: an instance of libckan.model.client.Client
>     :type client: libckan.model.client.Client
>     :param query: The search criteria.  See above for description.
>     :type query: string or list of strings of the form "{field}:{term1}"
>     [...]
>     :returns: the dictionary returned by the CKAN API,
>         with the keys "help","result", and "success".
>         "results" is a list of packages (dict).
>     :return: dict
>
>     Raises: :class:`libckan.model.exceptions.CKANError`:
>         An error occurred accessing CKAN API
>     """
>     # validation of parameters
>     args = client.sanitize_params(locals())
>
>     resp = client.request(action='resource_search', data=args)
>     if not resp['success']:
>         raise exceptions.CKANError(resp.error)
>     return resp
> ---
>
> - Function names semantically drive to the desired functionality. Imagine IDE auto-completion for "libckan.logic.action.res".
> - Providing the parameters in the function declaration enforces clarity and validation". Function parameters will have as default value the CKAN expected default values, instead of that empty string.
> - The Sphinx documentation clarifies in terms of what the function does/wants/returns/raises. Additionally, it lets me generate a separate libckan documentation.
> - Function arguments enable some validation on the client side, too. The sanitize_params() function is also for that.
>
> Custom exceptions help, although they might also exist with the ckanclient.post_to_ckan_api approach.
>
> The issue with custom actions can be solved.
> The first function parameter, client.Client(), is nothing else than an enhanced ckanclient.post_to_ckan_api('package_create', name='foobar').
> Client.request(action, data=None, base_url=_base_url, api_key=_key) achieves this.
>
> Here is my proposal: what about a hybrid approach?
>
> - CKAN "official" vanilla APIs defined as single functions
> - A libckan.logic.action.custom_request(client, action, args) function for custom actions and for developers not willing to follow libckan semantics and just provide the action as string.
>
> It seems this would solve the issues and even enable more freedom. Am I missing something?
> I might still drop the current libckan approach if that does not convince you (you all, not only Sean).
>
> --Daniel Graziotin
>
>
> _______________________________________________
> ckan-dev mailing list
> ckan-dev at lists.okfn.org
> http://lists.okfn.org/mailman/listinfo/ckan-dev
> Unsubscribe: http://lists.okfn.org/mailman/options/ckan-dev