Forum OpenACS Development: Calling ad_script_abort from within ad_returnredirect

Would it make sense to call ad_script_abort from within ad_returnredirect so that people don't forget to abort their scripts? Currently we do

ad_returnredirect $redirect_url
ad_script_abort

and after this change the second line would go away.

Collapse
Posted by Dave Bauer on
Don,

Is it possible for the script to continue to run after the redirect? That is, if some long running query or other process needed to run, is it possible to redirect the user so they don't have to wait, and then finish processing?

Collapse
Posted by Jeff Davis on
there are some places it needs to fall through (for example
in the registration sequence where it sends an email to
the admin about a new registration).  changing the default
behavior and adding a -no_abort flag would be ok though.
Ehem....ok, sorry Don, I happened to post this question as you (I swear, it wasn't intentional...) :-) Hmm, maybe I should stay logged in as Don... :-)

The short story of the matter is that Don arrived here in the office and he was jet lagged and went back to the hotel for a nap. Then all of a sudden Mohan told me Don was posting in the forums again. I was wondering how even someone like Don would manage to do that while sleeping in an unconnected hotel room until I realized that I was Don...

So, back to the issue at hand. I guess I'll try to make the change and add the -no_abort flag that Jeff suggested.

Collapse
Posted by Jeff Davis on
Peter, you will also need to check everywhere it is used to see if the no_abort flag is needed.
Jeff,
yes, of course, that's true. Given the sheer number of ad_returnredirect's in the code base I think I'll bail out from this change unless someone can find a fast and reliabel way to find where the no_abort flag should be added.
Collapse
Posted by Tom Jackson on

How about changing it for the future, and just add an -abort switch? It would also make the code easier to read, since the behavior is specified, instead of a blind default.

Collapse
Posted by russ m on
I'd have thought the reason for doing this is to make it harder for the programmer to mess up. Adding an optional -abort switch and requiring the programmer to remember to use it is no different to requiring the programmer to remember to use an ad_script_abort. (The -abort switch would have to be optional otherwise you're back to the problem of fixing every call to ad_returnredirect in the current sources).

Personally I like the idea of -no_abort since it makes the most common behaviour the default, but I'm not putting my hand up to crawl through every use of ad_returnredirect to see if it's required or not... :)

Not only does it occur a number of times in the toolkit, it is propably relied upon in a lot of custom code as well. Imagine how hard a change like this would make the life of those who are upgrading their existing sites with a new version of oacs. It would produce a failure of their custom code that would hardly be noticed immediately, and make poor sysadmins search for the cause in e.g. their mailing setups in case of an ns_sendmail call after ad_returnredirect. You wouldn't even be able to catch it with an automated testing script unless you can somehow check for successfully delivered mails with it.

I think since that it seems that OpenACS is actually being used we should take more care for those people who want to upgrade their code base from time to time, or at least not make their life unnecessarily hard.

A more acceptable solution would be to introduce a new command like ad_redirectandabort (think of a better name) and replace the occurences of ad_returnredirect in the toolkit with it, and maybe eventually deprecating ad_returnredirect, IMHO.

Collapse
Posted by Tom Jackson on

This change would be a mere convenience. Changing an api in an incompatible way for future convenience doesn't seem right. Either change the api in a compatible way, or create a new command, as suggested by Tilmann. My suggestion was for a default that produces the same behavior as the current command. I would prefer a new command to a change.

Tom, Russell, Tilmann,
thanks, those are all good points. I'll leave things as they are for now. Upgradability is an important issue indeed. Shame on me for not taking that into account...
No! The script is supposed to continue running after doing the redirect. Changing the default semantics of ad_returnredirect solely to "protect" incompetent programmers from themselves is an exceedingly bad idea. (As others here have pointed out.)
"Changing the default semantics of ad_returnredirect solely to "protect" incompetent programmers from themselves is an exceedingly bad idea."

I want to set the record straight here as Andrew's talk of incompetent programmers diverts attention from the important issues at hand.

The proposed change was to make the proc do what it should have done in the first place, behave as most programmers would expect it to behave, and most importantly behave as those programmers *want* it to behave in the vast majority of cases.

So to summarize, the proposed change would have been an improvement of our API, not only for incompetent programmers as Andrew suggests, but for the majority (or even all) programmers. Still, and this is important, we choose not to make the change since we take upgradability very seriuosly.

So am I making this post merely to prove that I am not an "incompetent programmer"? No, I also have another and more important agenda. Recent conversations with Mohan have helped me realize that throwing around terms such as "exceedingly bad" and "incompetent programmers" in the OpenACS forums does not help foster the friendly and inviting atmosphere that this community so sorely needs if it is to grow beyond the confines of elitism and arrogance that it inherited from ArsDigita. This means that senior members in the community, such as Andrew and myself, need to set a friendly, encouraging, forgiving, and even diplomatic tone, so that newcomers feel comfortable asking questions and contributing ideas.

Collapse
14: tone (response to 13)
Posted by Andrew Piskorski on
Anyway, I don't find it especially interesting, but since this new topic (tone of OpenACS Forums posts) started between Peter and I in email, and is at least potentially important (e.g., if I'm wrong below it could be very important), here's my contribution:

On Wed, Apr 09, 2003 at 02:32:33PM +0100, Peter Marklund wrote:
> Hey Andrew!
> I take your point, and it's good. However, please note that this kind of
> tone in our forums does not invite beginners to ask questions. Mohan

[shrug]  Well, you're entitled to that opinion.  My tone is usually
much more helpful, even (especially?) to clueless newbies who don't
know what they're doing, as you can see from my posting history.  In
this case, the tone seemed justified to me, even necessary.  Why else
would I post an essentially redundant comment, after all?  For
emphasis.

> Pakkurti has talked to several developers in Europe who have confirmed
> that this is a real problem that we have.

Rumor and hearsay.  If we're going to go by anecdotal evidence (which
in this case is probably fine), we should at least know the full
details of the anecdote.  And any such discussion probably belongs in
public on the BBoard, not here.

Tone and culture cut both ways.  I would hope that NO developers
trying to use and/or contribute to OpenACS consider themselves
"incompetent".  If they do, well, I don't think we want them, do you?
(Of course, the truly incompetent never consider themselves such, but
that's straying of topic...)             

Further, IMNSHO, the OpenACS BBoards show an unusually high degree of
both competence and helpfullness on the web.  High signal to noise
ratio.  That's why I like it so much.  I could be wrong, but in
general I don't see any "tone" problem.