Ckan: Conflicts between names/interfaces of IPackageController and IResourceController

Created on 11 Apr 2016  路  7Comments  路  Source: ckan/ckan

As of CKAN 2.5.2 the plugin interfaces IPackageController and IResourceController have conflicting method names and method interfaces. For example, both interfaces have a after_delete(context, obj) method, but according to the documentation, obj is a package dict for IPackageController and _a list of_ resource dicts for IResourceController.

This leads to problems when a plugin implements both interfaces, since one then needs to do a lot of (un-Pythonic) type checking in each of these methods.

Suggested solution: Deprecate the existing method names and add new ones with scoped names (e.g. after_package_delete and after_resource_delete). If possible try to make the interfaces consistent where it makes sense (e.g. in the case of after_delete) to avoid confusion.

All 7 comments

Are there any updates on this? I'm using the "after_create" interface for both resources and dataset in my extension for validation checking. The only way to avoid weirdness is to, as @torfsen says, type check the dict as it comes in. (I don't even know how to do this, do I have one "after_create" method that potentially is called by both interfaces?)

This creates unnecessary overhead and is quite hard/annoying to debug.

I guess I will just check for the "size" parameter as resources will have a size, and packages will not.

@NicolaiMogensen I don't think anyone is currently working on this, so for the time being the workaround you describe is the best way to go.

If anyone else stumbles upon this, this is what I did

# IPackageController / IResourceController
    def after_create(self,context,pkg_dict):
        if "size" in pkg_dict:
            log.debug("The updated dict was a resource")
            #Put code here
        else:
            log.debug("The updated dict was a package")
            #Put code here

I found that for after_update both the package and the resource handler gets called sequentially.

We decided to close old issues that are not actively worked on so that we can focus our effort and attention on issues affecting the current versions of CKAN.

If this issue is still affecting the version of CKAN you're working with now, please feel free to comment or reopen the issue.

If you do reopen this issue, please update it with new details. One reason it might not have been resolved in the past is that it wasn't clear how a contributor could address the issue.

It is still an issue in my opinion; it should be quite easy to fix it (just use different function names, call the old ones for compatibility reasons, raise a deprecation warning, update the doc).

Yeah, good for contribution. @wardi said:

how about after_resource_update and similar.
just need to handle falling back to the old method names for backwards compatibility.
was thinking just a hasattr check on each plugin before calling the function.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

amercader picture amercader  路  5Comments

TkTech picture TkTech  路  4Comments

lperepol picture lperepol  路  8Comments

metaodi picture metaodi  路  6Comments

LeonanCarvalho picture LeonanCarvalho  路  3Comments