[ckan-dev] extensions decoupling: ckan.model, get_action & metadata

[Toby Dacre]

On 20 June 2012 12:13, Adrià Mercader <amercadero at gmail.com> wrote:

> Hi,
>
> > as nobody has objected I'll assume this is all good
> Sorry, I hadn't finished my reply! (We should at least allow 24 hours
> for replies, there are still 2 left :) )
>

Ok I'll agree to be over keen on this but it did provoke feedback ;)

>
> While I agree that trying to separate extensions from core is the way
> to go, maybe stats is not a particularly good example, as it is
> perhaps too simple.
>

simple is a good start


>
> Correct me if I'm wrong but you seem to suggest that extensions won't
> be able to import the model. Are we sure that extensions can access
> all necessary information just with the Session? Extensions use the
> metadata object to create tables or import domain objects
> (model.Package, model.User, ...) for various things. Would we be
> restricting all of this?
>
> we discussed this a bit at the dev meetup.  The problem was that giving
access to model ties our hands quite a bit for example the Package.to_dict
function may get removed - kindly is keen to do this or at least was at one
point.  If it's available via plugins.toolkit we promise to not break the
`api` so I don't want to do that.
Plugins can still just import ckan.model but if they do this then we do not
agree not to break them.  If they connect via plugins.toolkit then they
should not be breaking however much we refactor core
So anything in toolkit becomes a long term support commitment so we need to
be careful


> Some random examples:
>
>
> https://github.com/okfn/ckanext-dgu/blob/bigbang/ckanext/dgu/controllers/publisher.py#L206
>
> https://github.com/okfn/ckanext-harvest/blob/master/ckanext/harvest/model/__init__.py
>
> https://github.com/okfn/ckanext-spatial/blob/master/ckanext/spatial/model.py
>
> We should arrive to a compromise between decoupling from core and
> allowing extensions flexibility.
>

hopefully I've cleared this up but let me know if i failed

>
> On 19 June 2012 14:04, Toby Dacre <toby.okfn at gmail.com> wrote:
> > Hello,
>
> > 2) logic.action.get_action() is available via
> > ckan.plugins.toolkit.get_action however there is a problem that many
> actions
> > require {'model': ckan.model} as context.  To get around this my thought
> is
> > to create a function ckan.plugins.toolkit.call_action(action_name,
> context,
> > data_dict)  that would call the action but add 'model':ckan.model to the
> > context if not supplied.  We could make that optional via an extra param
> but
> > I think that just adds complication and I can't see a side effect.
> Sounds good to me
>
> > 3) lastly the stats extension uses sa.select() to access table data for
> this
> > it needs access to either the model.meta.metadata or a function to return
> > the correct sa.Table I'm not too fussed which way we go with this do
> people
> > have any opinions
> >
> > 4) do we make BaseController available via the plugins.toolkit?
> I'd say yes, again harvest and spatial use it (or the ApiController)
>

thanks for the feedback

>
>
> Adrià
>
>
> >
> > Toby
> >
> >
> >
> >
> >
> >
> >
> > _______________________________________________
> > ckan-dev mailing list
> > ckan-dev at lists.okfn.org
> > http://lists.okfn.org/mailman/listinfo/ckan-dev
> >
>
> On 20 June 2012 11:59, Toby Dacre <toby.okfn at gmail.com> wrote:
> >
> >
> > On 19 June 2012 14:04, Toby Dacre <toby.okfn at gmail.com> wrote:
> >>
> >> Hello,
> >>
> >> I've been looking at the stats extension today and looking to make it as
> >> independent from core as we can.  There are some issues that I think
> need
> >> some discussion.
> >>
> >> 1) from our dev meetup we agreed that we don't really want to have
> >> extensions getting hold of the model directly as these are liable to
> >> change.  We did however agree that they do need access to model.Session
> so
> >> my plan is to make model.Session available via
> >> ckan.plugins.toolkit.ckan_session (ckan_session feels better that
> Session as
> >> it is an object not a class but this can change if people have strong
> >> feelings)
> >>
> >> 2) logic.action.get_action() is available via
> >> ckan.plugins.toolkit.get_action however there is a problem that many
> actions
> >> require {'model': ckan.model} as context.  To get around this my
> thought is
> >> to create a function ckan.plugins.toolkit.call_action(action_name,
> context,
> >> data_dict)  that would call the action but add 'model':ckan.model to the
> >> context if not supplied.  We could make that optional via an extra
> param but
> >> I think that just adds complication and I can't see a side effect.
> >>
> >> 3) lastly the stats extension uses sa.select() to access table data for
> >> this it needs access to either the model.meta.metadata or a function to
> >> return the correct sa.Table I'm not too fussed which way we go with
> this do
> >> people have any opinions
> >>
> >> 4) do we make BaseController available via the plugins.toolkit?
> >>
> >> If we resolve these issue then it should be possible to make stats
> >> independent of core which feels like a good direction to take on the
> general
> >> extension cleanups.
> >> the current progress if anyone is interested is in branch
> >> enhancement-2572-clean-stats-plugin
> >>
> >> The remaining decoupling issues after that relate to the tests but I was
> >> going to look at them once these issues are resolved and I'll post about
> >> them separately.
> >>
> >
> > as nobody has objected I'll assume this is all good
> >
> >>
> >> Toby
> >>
> >>
> >>
> >>
> >>
> >>
> >
> >
> > _______________________________________________
> > ckan-dev mailing list
> > ckan-dev at lists.okfn.org
> > http://lists.okfn.org/mailman/listinfo/ckan-dev
> >
>
> _______________________________________________
> ckan-dev mailing list
> 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/20120620/e9c00a1e/attachment-0001.html>