Forum OpenACS Improvement Proposals (TIPs): Tip #26 (Approved for 5.1): Exchange acs-mail-lite

I propose to exchange acs-mail-lite from /packages with the version in /contrib/packages due to the fact that the latter contains improvements in mail sending and bounce management. No datamodell change has been made. Furthermore documentation has been added, including steps on how to setup postfix. I propose this change well aware of the Release efforts. It is up to the release team to postpone this.
Collapse
Posted by Tilmann Singer on
These additions are cool (for sure, haven't looked in detail yet though), but if we do change the toolkits mailing system anyway then it'd make sense to clean up the current confusing situation of having two different packages handling similar functionality, of which one has a silly name, by merging your new acs-mail-lite into acs-mail and use only that henceforth.

If done in one step then it will be easy to communicate as 'The new acs-mail' instead of having several interim stages, which reminds me on the confusion of photo-album (is the new version now part of MAIN? etc.).

Collapse
Posted by Lars Pind on
Malte,

Does the contrib version of acs-mail-lite contain all the changes that has happened to the packages version of acs-mail-lite, since you forked it?

/Lars

Collapse
Posted by Malte Sussdorff on
We forked from the version Mat Kovach provided, at least this is what we have been using. The difference from a first look at diff is that you changed it on 2nd of September, adding a check that subject contains no new lines (will be added in a few minutes) and a catch around the sending of queued items. We solved this a little bit differently, so not sure if we need to incorporate your code if the result is the same.

But maybe Tilmann is right, and we should go and take out the mail system confusion first. Which would lead me to the question, why do we have acs-mail and acs-mail-lite in the first place and what packages are using acs-mail. I mean, if we get rid of one of them my understanding is that we have to rename the mail-procs in each package, or am I utterly mistaken here (I admit, no look at the acs-mail code yet).

Collapse
Posted by Lars Pind on
Yes, we'd have to change existing code to use whatever we decide to standardize on.

One of the major differences between acs-mail and acs-mail-lite is that acs-mail handles multipart MIME messages, encoding/decoding each part appropriately based on the content type, etc.

We need this functionality.

Does your acs-mail-lite support this?

/Lars

Collapse
Posted by Tilmann Singer on
<blockquote> why do we have acs-mail and acs-mail-lite in the first place
</blockquote>

My guess the only reason is this: Because openforce was under heavy time pressure when they had to deliver dotlrn and didn't want to bother with understanding and potentially having to fix the existing package acs-mail.

Don't know what packages are using acs-mail right now, but if we merge acs-mail-lite into it that would mean that at least we would have to change the calls in all packages that use acs-mail-lite, which is mainly notifications. And maybe some more in dotlrn.

Maybe the merge would also suggest improving / modernizing the acs-mail api which would mean that existing acs-mail calls would need to be modified as well (I'd volunteer for that). Or we can build a compatibility wrapper that maintains the old acs_mail_lite:: and acs_mail calls.

Collapse
Posted by Malte Sussdorff on
Hi Lars, define *support*. What we do in the mailinglist package is to use a special send function, that handles multipart MIME, encoding/decoding, alternative mails, character sets (okay, maybe I go to far there).  And acs-mail-lite sends it out. So yes, for sure, it does support this ;).

If you want this to be part of the mail system it should not be much of a deal to just copy the function from one scope to another. But here I will stay out and ask Timo to comment on this, as he was heavily working with it and has the in depth knowledge.

Collapse
Posted by Randy O'Meara on
I just thought I'd reiterate again...

PLEASE DON'T BREAK EXISTING CODE

Sorry for raising my voice, but there has to be a better way to migrate to a newer and better mail package than ripping the old one out (moving it to the obsolete packages graveyard) and plunking in a new one with the same name but a different API.

There are folks out there using the current packages and they will not follow along to new releases if it breaks their production code.

Or, maybe I misunderstood what is proposed?

Collapse
Posted by Tom Jackson on

Randy, I don't think you misunderstood at all.

You know HIV, the virus that causes AIDS, kills its host by constant mutation. Our amazing immune systems kill most of it over and over again, but a little bit that is slightly different remains to grow up again. API changes are like giving HIV to your Open Source project. Amazing, enthusiastic programmers do their best to keep up, but eventually succumb to these tiny changes.

But I'm also reminded of how, when I was a kid, I would find a typewriter or other complex mechanical device. Not having the tools or patience to take it apart, I'd just smash it on the ground and then collect the parts that fell off. Cool!

Collapse
Posted by Malte Sussdorff on
Sorry to contradict Tom here, but I'm not talking about breaking existing code. If the OCT wants to move my initial proposal of upgrade of acs-mail-lite to a overhaul of the mailsystem, that is up to them. I merely propose to use the contrib version of acs-mail-lite instead of the version that resides in packages, as the contrib version contains quite some *enhancements* that are *not* breaking any existing code.

So maybe we should stick with the *original* TIP and make a decision on this before we talk about changing the whole mailsystem. But again, this is up to the OCT.

One note on the API changes. If I'm not mistaken, mutation is welcome in the world of science, otherwise progress would not happen. What we just have to make sure is that the API does not break existing code (thereby not taking a darwinistic point of view). Which is really easy to accomplish, especially in this case.

Anyway, all I can do  is to mention that there is acs-mail-lite in contrib, that does not break existing code, is compatible to the version in packages and it's up to the OCT to come to a decision whether to use it for OpenACS or not. Anyone wanting to get bouncing and documentation can still take the contrib version after all.

Collapse
Posted by Don Baccus on
acs-mail, as written today, does not scale.  That was the main reason for OF writing acs-mail-lite.  It creates all sorts of objects to contain MIME attachments and stuff (I think, I haven't looked at this in a long time) and overall the implementation just seemed very inefficient.

And it was only used in a few places.

While I'm sympathetic to the goal of backwards compatibility, do we really need to keep the worst of the worst available for use forever and forever when we know the code really sucks and is fubar'd beyond any reasonable hope of repair?

How devoted are you to this concept, Tom?  If we uncover a security hole that requires an API change to fix, should we leave the hole open because of slavish devotion to the principle of maintaining API compatibility forever?

I wouldn't see any harm in deprecating acs-mail in favor of an acs-mail-lite that has all the features we need but is more efficient as long as we give people ample notice that it's going away.  What is ample notice, then?  Six months?  A year?

Collapse
Posted by Don Baccus on
As far as Malte's proposal goes ... having these improvements in acs-mail-lite would be good but it's beyond the feature freeze date and ISTM we should wait until 5.1 ...

Comments?

Collapse
Posted by Dirk Gomez on

Ample notice in the world of Oracle Apps is 2 releases.

6 months is a good time and lets involved parties investigate changes properly and thoroughly.

If we let bad code linger around forever, we encourage junior *and* senior OpenACS hackers to copy bad behaviour. A lot of the code written is copied/taken from already existing code, the worse the original, the worse the newly-built code.

Collapse
Posted by Jeff Davis on

One thing I think is important to do is make clear which packages are no longer actively developed and which have been superceded by another package. Moving them to obsolete-packages seems to be the best way to do that. bboard is a good example. People were still using it on new installs even though it had a host of problems mostly because it wasn't clear that forums was the prefered package.

In any case, I had a look through the code and here are some observations:

  • acs_mail_lite::with_finally should go somewhere else (probably along side with_catch in acs-tcl utilities-procs.tcl)
  • it needs upgrade scripts for existing installs (since it changes the datatype of to_addr to text/clob, and adds some columns and renames some constraints
  • I dislike that it renames existing constraints since its certainly possible that some people code might check for the contraint name when an insert or update fails.
  • the code has tabs rather than spaces (although since we do not have any coding standards I guess thats not a fair criticism).

I agree with Don that it should be a 5.1 thing though.

Collapse
Posted by Tom Jackson on

I think Malte's original TIP is a good idea, but the future of mail, combining two mail packages, changing the bad api and updating the rest of OpenACS and dotLRN (forgetting client code) in one TIP is a little much. When someone says "I approve" to this TIP, what does it mean?

Collapse
Posted by Lars Pind on
Approved
Collapse
17: Re: Tip #26 Seconded (response to 1)
Posted by Caroline Meeks on
Approve
Collapse
Posted by Tilmann Singer on
Agreee with Tom, it would be too much to transform this TIP into a huge merge-and-repair all, so I'll label my suggestion as such, definitely not a veto. Anyway, this shows that it would be beneficial to have a forums discussion before posting a TIP so that things like these can be sorted out before.

On the topic of not breaking existing code - yes of course. A merge would have to consider upgrade issues as well, maybe someting like a compatibility api would be feasible.

Collapse
Posted by Joel Aufrecht on
Okay, we've got two weeks before the proposed code freeze for 5.1.  Is this going to happen or does it need to be bumped to 5.2?
Collapse
Posted by Caroline Meeks on
FYI I am using it on a 5.0 production site I just launched.

I've got an error in the logs with the bounch processing that I will fix in the next few days.

Other then that it seems to be working for me.

Collapse
Posted by Dirk Gomez on
Does it work properly with Oracle? See my fix at http://cvs.openacs.org/cvs/openacs-4/packages/acs-mail-lite/sql/oracle/acs-mail-lite-create.sql I just fixed the create-table statements didn't test the package's functionality though.

Is it maintained?

It doesn't have *any* documentation at all, even the code is sparsely documented. Does it need doco?

Documentation has been commited to HEAD. The exchange has happened on HEAD. I tried to remove it from contrib on HEAD. Dirk's fixes have been added. It is as maintained as the previous version (if something breaks, it is a showstopper and someone will fix it. For the moment that is me). If the documentation does not make sense to you or you stumble across any problems in setting up bounce-management, feel free to ask and thereby help to improve the documentation.