Forum OpenACS Development: Re: upgrade-5.9.1d15-5.9.1d16.sql breaks acs_rel_type__create_type

Collapse
7: Re: upgrade-5.9.1d15-5.9.1d16.sql breaks acs_rel_type__create_type (response to 6)
Posted by Antonio Pisano on 12/30/20 07:17 PM
Dear Frank,

we normally take care of not breaking compatibility with old code and we go some distance in order to achieve this: supporting legacy flags, providing fallback implementations for new api... and so on.

Of course there could be much more use cases in the wild that we sometimes assume, plus various mixtures of legacy/custom code and setups.

In this particular case, the change was quite subtle, as it would affect the type of the argument, rather than the argument itself, so the chances increase that some usage would break.

That people find and report issues is perfectly normal and healthy for a project. What is even healthier is contributing so that issues are fixed and/or detected earlier. Hence my suggestion, if you feel like offering some help, would be to write new automated tests, possibly involving the API you use the most or care the most about.

Running such tests, together with the ones already available, on a VM of yours on a regular basis (e.g. in a continuous integration pipeline) would also be much appreciated, as there are certainly features of your setup that might be different from, say, those on LEARN or a vanilla instance.

Happy new year to you too!

Ciao

Antonio

Collapse
8: Re: upgrade-5.9.1d15-5.9.1d16.sql breaks acs_rel_type__create_type (response to 7)
Posted by Gustaf Neumann on 01/03/21 10:38 AM
"too many" broken API calls without good reason in the last 2-3 years.
hmm, you are aware, that the discussion about the issue on hand goes back to 2003 (17years). The proposed fix was out since 2013 (7 years ago) and was committed finally in 2016. There was plenty of time to look at the changes and to consider consequences for other projects. ... Also be aware that this change was requested by users (as most of the changes).

At the time of the commit, the final solution was the best compromise between keeping high backward compatibility and functional extension. I did these changes for all of the ~100 maintained packages, the changes were very little and took me just a few minutes.

If you look at this change, it was necessary to introduce an additional parameter to the stored procedure (in PostgreSQL and Oracle). To keep the call highly compatible, the parameter was made optional. This works without maintenance cost for most of the cases, but due to the fact that there was before the change already versions of acs_rel_type__create_type with 15 and 16 arguments, with very similar argument types, it required casts in rare situations, where the types cannot be inferred from the values (in particular "null") for function disambiguation.

If one compares OpenACS with other frameworks (php, ruby, ... JavaScript) the stability of the interface is one of the highest around. Most compatibility problems are from sites, using deprecated interfaces, non-public interfaces, or local modifications. Often the sites are not aware of the deprecation. This improved over the years, since modern OpenACS provides log entries when deprecated API calls are used.

Part of the problem is as well that there is no API for "acs_rel_type__create_type" (a call, which does not exist as such on Oracle).... But even when we would introduce an API for this, using the API would require maintenance work. There are no calls deliberately altered. Backward compatibility is a high goal (but not the only one).

The OpenACS core team was always and is always open for collaboration. Collaboration requires effort from all sides, following what is happening in the community, reading commit messages, etc.

We are very, very happy that there is active OpenACS development from your side
Sometimes it is not so easy to read between the lines.

All the best in 2021!
-g