[ckan-dev] allow_partial_updates - issues with demo site - where should we be going?

[Haq, Salman]

I realize I'm late to thread but wanted to share my thoughts anyway.

That allow_ partial_updates exists is a symptom of a larger problem, namely that the Package controller is monolithic.

I concluded this after developing my own extension that introduces a special kind of Resource with its own data model and form.

During this exercise, I had to override three action functions via IActions,  implement a new controller (from BaseController), implement IPackageController, IDatasetForm, ITemplateHelpers and override three templates inside the package/ folder.

I did all this just so I can display a new sub-nav menu item and allow the user to CRUD this special Resource. The Package, Resource, and Revision concepts are too closely intertwined in the code. Having said that, I have been able accomplish almost all the features I wanted to implement. The only exception has been the inability to purge deleted Packages because revisions associated with my extension's model cannot be deleted (see previous email about 'purging revisions'). The latter issue could be fixed by introducing a new kind of entry point method in the CKAN core.

Just some food for thought. I'm not advocating doing anything drastic based on this perspective.

Thanks,
Salman

From: Toby Dacre <toby.okfn at gmail.com<mailto:toby.okfn at gmail.com>>
Reply-To: CKAN Development Discussions <ckan-dev at lists.okfn.org<mailto:ckan-dev at lists.okfn.org>>
Date: Friday, July 13, 2012 11:56 AM
To: CKAN Development Discussions <ckan-dev at lists.okfn.org<mailto:ckan-dev at lists.okfn.org>>
Subject: Re: [ckan-dev] allow_partial_updates - issues with demo site - where should we be going?

I've created a branch to fix this behaviour when a config option is set

https://github.com/okfn/ckan/pull/67

On 13 July 2012 11:24, Toby Dacre <toby.okfn at gmail.com<mailto:toby.okfn at gmail.com>> wrote:


On 13 July 2012 11:11, Sean Hammond <sean.hammond at okfn.org<mailto:sean.hammond at okfn.org>> wrote:
> With the demo site we are using package_update action
>
> we are using allow_partial_updates when we are doing the updates - this is
> great for things like not loosing all our resources when we update a
> package however it also means that we cannot for example remove all our
> tags as an empty tags is seen as non-existent and so the tags are not
> updated
>
> What I would prefer is that tags is in pkg_dict we would update it but if
> it wasn't we wouldn't

Yes I agree, for everything in dictionaries. It's a PITA to have to pass
the entire dict back to the CKAN API to avoid it deleting random bits
that you're not interested in. If I just want to change a package's
tags, for example, I should just be able to call package_update passing
only the package ID and the tags, and this should not cause the
package's resources (for example) to get deleted.

> the problem with this is that form data is validated and an empty tag is
> removed from that data - then when we are in the update missing tag data
> ends up becoming [ ]

You mean that if the dict that I pass to package_update has no 'tags'
key at all, then this deletes all the package's tags? I agree this
shouldn't change the package's tags at all, but it sounds like you're
going to have to modify the validate code to fix that, and that could be
hairy.

The other thing is, you would be changing the behaviour of the API (all
the *_update functions I think) so you might break clients.

How about a config option to change the behaviour?  I'll start by doing a branch and seeing if all the tests still pass that would be a good sign of things - if some fail then I think the config option would seem a way forward.  This isn't going to happen till 1.9 if it is accepted.


> It seems to me that tags = [ ] should not be seen as empty by validation
> whereas no tags in the data should be

Don't you have this backwards? tags=[] in the data _should_ be seen as
me trying to remove all of the package's tags, no 'tags' key in the data
should mean don't make any change to the package's tags.

Sorry my language may be a bit confused. I mean is that if I pass [ ] to the validation it thinks I've passed nothing and so removes it and then later in the process the lack of something then gets changed to [ ].

What should happen is that [ ] does not get killed by the validation and a missing value does not get converted to [ ]

> then when processing the update we should not convert None -> [ ]

Yes

Thanks for the feedback

> The other option would be for me to add a more modular
> allow_partial_updates which would allow the items to be updated/or not to
> be more granularly controlled but this would become messy and the code
> would look shitty

Yes this sounds complicated

_______________________________________________
ckan-dev mailing list
ckan-dev at lists.okfn.org<mailto:ckan-dev at lists.okfn.org>
http://lists.okfn.org/mailman/listinfo/ckan-dev


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.okfn.org/pipermail/ckan-dev/attachments/20120718/de04de6f/attachment-0001.html>