Forum OpenACS Development: RFC: Callbacks

Collapse
Posted by Andrew Grumet on
At various points we've talked about adding a standard way to do callbacks in OpenACS.

The idea is to give Core the ability to invoke code in non-core packages that may or may not be installed, but without cluttering core with an ever-growing list of "if" statements that check if package X is installed before calling its code.

I've written up a short proposal at

http://grumet.net/writing/programmer/openacs/callbacks

The guiding design principles are

* It should be easy to find or create a page of all available callback entry points.
* Don't create a new documentation mechanism -- extend ad_proc instead.
* Encourage programmers to put implementing code in separate namespaces, to avoid collisions.

Collapse
2: Initial comments (response to 1)
Posted by Andrew Grumet on
There is some overlap with service contracts. The callbacks are meant to be simpler, not involve the database, be geared towards single operations rather than grouped contracts, and come with a built-in method for invoking all implementations.

Implementing this is pretty simple -- the proposal has some initial working code in it, and at the bottom of the proposal are some suggestions from Jeff that make it even simpler.

Before TIPping I thought I'd put this out for discussion again, and get feedback on how this should be exposed.

One question that's come up is how to organize the namespaces. For example I proposed that the implementation proc should look like this:

   ad_proc -private ::dotlrn::callback::acs_user::changing_state {

where "dotlrn" is the home package/module of the implementing code, "callback" is a separator used by the callback system, "acs_user" is the home package/module of the callback entry point, and "changing_state" is the operation name.

Jeff suggested an alternative:

   ad_proc -private ::callback::acs_user::changing_state::dotlrn {

or separator/callback entry module/operation/implementor.

This alternative has the advantage of simplifying the callback system's introspection code, but it means that package implementors are building code in other namespaces besides their own. That may be appropriate given the nature of callbacks.

I don't have much of a preference, actually, but I'd like to hear more thoughts on this and the rest of the proposal.

Collapse
3: Re: RFC: Callbacks (response to 1)
Posted by Dave Bauer on
I like this proposal. Personally I prefer the second naming convention where all procedures that implement a particular callback are in the same namespace.
Collapse
4: Re: RFC: Callbacks (response to 1)
Posted by Jeff Davis on
I would mention that besides my way being easier to write the necessary introspection code, it will also perform better as it does not have to walk the namespaces (which I think can be an important consideration here.

We could also make the signature be

ad_proc -callback acs_user::change_stat -implementation dotlrn  { ... }
and have the namespace we end up giving it be an implementation detail not exposed to the user.
Collapse
5: Re: RFC: Callbacks (response to 1)
Posted by Jade Rubick on
I really like this proposal. I think the simplicity of this API vs. service contracts is a big plus -- I'd use it.
Collapse
6: Re: RFC: Callbacks (response to 1)
Posted by Andrew Grumet on
Okay, so we'll use the second style for collecting implementations into namespaces.

ad_proc -callback acs_user::change_state -implementation dotlrn { ... }

I like it, but just to be clear, we'd be asking them to put these in namespaces, e.g.

namespace eval dotlrn {

    ad_proc -callback acs_user::change_state -implementation dotlrn  { ... }

}

Right? The alternative, asking programmers to use the same fully qualified function name repeatedly across the toolit, seems like a problem waiting to happen.

If we can require that the callback implementations are declared in namespaces, we could conceivably drop the <strike>-callback</strike> -implementation flag and introspect for it.

Collapse
15: Re: Re: RFC: Callbacks (response to 6)
Posted by Andrew Piskorski on
Please do not define Tcl procs via the "namespace eval foo { pages_of_procs_here }" mechanism. That makes finding the procs in the files via Emacs or ctags unreasonably difficult. (And it also adds an unnecessary and annoying additional level of indentation.) Instead, simply define each proc using its fully-qualified name, e.g.:
ad_proc -callback dotlrn::acs_user::foo::bar { ... }

As far as the actual topic of this thread - yes, good OpenACS callback support would be excellent, I applaud this effort. I have no particular thoughts or feedback on it myself, but between Jeff and Andrew working on it I'm pretty sure you'll end up with something good. :)

Collapse
7: Re: RFC: Callbacks (response to 1)
Posted by Andrew Grumet on
Oops, I meant to say we could drop the -implementation flag.
Collapse
8: Re: RFC: Callbacks (response to 1)
Posted by Jeff Davis on
Andrew: the way I thought it would work is that
ad_proc -callback acs_user::change_state -implementation dotlrn  { 
... }
would create a function:
::__callback::acs_user::change_state::dotlrn {}
(i.e. in the __callback namespace), and that you would invoke via:
callback acs_user::change_state args
or
callback -implementation X acs_user::change_state args
It would be rooted in a namespace where no one should be using.
Collapse
9: Re: RFC: Callbacks (response to 1)
Posted by Jeff Davis on
Also the reason I thought we should have the -implementation flag is because I thought the way we would document and define the contract for a callback would be simply:
ad_proc -callback module::op { 
 {-option {} }
 arg1:required 
} {
 docs 
} -
That - at the end is ad_procian for {just docs no body}.

I was also thinking that we could even drop the arg definition for things that are

ad_proc -callback module::op -implementation X
and use the generated arg parser from the callback definition rather than having to repeat it again.
Collapse
11: Re: RFC: Callbacks (response to 1)
Posted by Andrew Grumet on
Yep, I got it. It all looks good, with one caveat.

So somewhere in core, we have a file containing the contract definition

ad_proc -callback module::op {
  {-option {} }
  arg1:required
} {
  docs
} -

Then in a module somewhere, someone defines a callback function but makes an error:

ad_proc -callback module::op {body}

They've forgotton the -implementation flag. Now it looks like a contract definition. How do we handle this?

Collapse
12: Re: RFC: Callbacks (response to 1)
Posted by Jeff Davis on
We could barf if there was a callback definition with a proc
body.
Collapse
13: Re: RFC: Callbacks (response to 1)
Posted by Andrew Grumet on
One answer is, "that's the programmer's problem". I don't like especially like that solution.

What if we required add an extra -contract flag for the person documenting the contract?

Collapse
14: Re: RFC: Callbacks (response to 1)
Posted by Andrew Grumet on
We could barf if there was a callback definition with a proc body.

This works too. Okay, I think we're pretty close.

Collapse
10: Re: RFC: Callbacks (response to 1)
Posted by Jeff Davis on
also it might be a good time to consider -pre and -post
precondition and postcondition flags for doing
contract enforcement.
Collapse
16: Re: RFC: Callbacks (response to 1)
Posted by Jeff Davis on
One comment on the etags issue:

For one thing invoking things via a callback means you don't know the terminal bit (implementation name) anyway since say for user delete you have:

callback user::delete $user_id
in the code which introspects all the implementations of the user::delete callback and calls them. You have to just know the implementation name you are looking for anyway. find-tag is not going to be able to guess you want to look at user::delete::forums since forums just isn't there.

Maybe we should make the above

callback::user::delete $user_id
since I agree being able to findtags the contract definition would be useful and saying callback:: instead of callback{space} is pretty unimportant.

The proc declaration would not be an issue either way:

ad_proc -callback acs_user::change_state -implementation dotlrn  { ... }
with etags (simplified to make things clear):
etags --lang=none --regex='/ad_proc -contract \([^ ]+\) -implementation \([^ ]+\) /callback::\1::\2/'
produces
x.tcl,110
ad_proc -callback acs_user::change_state -implementation dotlrn ##callback::acs_user::change_state::dotlrn,8
which would seem to work fine.
Collapse
17: Re: RFC: Callbacks (response to 1)
Posted by Eduardo Pérez on
I'm up for that if the implementation you suggest would get totally rid of Service Contracts in OpenACS.
I'm asking because you mention it as an alternative to Service Contracts instead of what would get rid of Service Contracts.

Is Signal a better name rather than Callback? (Just to not confuse people)