[ckan-dev] API refactoring and changes

Thu Feb 28 17:22:11 UTC 2013

Hi,

thanks for your feedback so far. I haven't changed any parameter names. For `current_package_with_resources` I marked `page` as deprecated and added an offset parameter. `page` is still supported. The only breaking changes that I regard as breaking are slightly different errors. However, this should not affect applications because they should not even run into these errors. Do you agree on this point?

@toby I totally agree that we should not change any parameter names. I will only change the way we do validation. 

Dominik

On 28 Feb 2013, at 14:25, David Read <david.read at hackneyworkshop.com> wrote:

> 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
>> 
> 
> _______________________________________________
> 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