[ckan-dev] API refactoring and changes

David Read david.read at hackneyworkshop.com
Thu Feb 28 13:25:06 UTC 2013


Dominik,

Hooray for flushing out lots of potential 500 errors in the API! And I
don't think anyone can say it is a breakage by converting a 500 error
into a 409 - it is simply an improved error indication.

Changing parameter names - I think you should allow the old names as
an alternative, to avoid the breakage.

David

On 28 February 2013 12:20, Toby Dacre <toby.okfn at gmail.com> wrote:
> Dominik,
>
> Is this api version 4?
> I am a bit concerned about the whole backwards compatibility thing.
> I'm happy for a code cleanup and less breakages but stuff like changing page
> to offset feels dirty unless we do it nicely
>
> Maybe I'm a bit overprotective - but also this is why we should add things
> slowly and properly and not rushed :)
>
>
>> One thing to discuss is whether we want to have basic validation for
>> **all** action functions in `controllers/api` instead of in every single
>> action. This would require a parameter schema for every action function.
>
> This feels totally wrong the actions should do all the validation not the
> api controller it should just pass things back and forth and maybe do a bit
> of compatibility stuff eg changing page to offset for ver 3 calls
>
>
>
>
> On 28 February 2013 11:37, Dominik Moritz <dominik.moritz at okfn.org> wrote:
>>
>> Hi Everyone,
>>
>> I am currently working on refactoring of the action API and making it more
>> stable. There are a few changes that might be controversial so I'd like you
>> to comment on them at [1]. I will update the comment as I commit code to the
>> branch. Please write your comments in the issue so that it easier for me to
>> keep track of them. Thanks.
>>
>> Here is a summary of the changes so far:
>>
>> ---
>>
>> * Many actions do not produce a server error any more if `limit` and
>> `offset` is a negative integer or no integer at all. Also validation for
>> other parameters. Kind of **breaking** because instead of a 500, you get a
>> 409.
>> * `ParameterError` was removed in favour of `ValidationError` **breaking**
>> * `_get_or_bust` goes through the validation code (common code paths)
>> * `_get_or_bust` returns a list instead of a single string for a missing
>> field (consistent with validate) **breaking**
>> * Updated docstrings
>>   * Datastore docs indicate optional parameters
>> * `current_package_list_with_resources` supports `offset`, `page` is
>> deprecated
>> * Auto complete actions get consistent parameters, `q` is now required in
>> all auto complete actions **breaking**, all actions support limit
>> * Added tests for changes
>> * `[package|resource]search` have proper parameter validation
>> * small fixes...
>>
>> There are still many functions that have no proper validation. Often bad
>> input is not tested and adding validation takes time because new validation
>> definitions have to be written.
>>
>> One thing to discuss is whether we want to have basic validation for
>> **all** action functions in `controllers/api` instead of in every single
>> action. This would require a parameter schema for every action function.
>>
>> ---
>>
>> Best wishes,
>> Dominik
>>
>> [1] https://github.com/okfn/ckan/pull/473
>> _______________________________________________
>> 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
>
>
>
> _______________________________________________
> 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
>




More information about the ckan-dev mailing list