[ckan-dev] test performance and sqlalchemy complete

Thu Jan 6 16:56:53 UTC 2011

On 6 January 2011 16:12, William Waites <ww at eris.okfn.org> wrote:
> * [2011-01-06 14:28:03 +0000] Seb Bacon <seb.bacon at gmail.com> écrit:
>
> ] I've commited a monkeypatch now.
>
> please roll that back and just put a patched sqlalchemy up and refer to it in the pip requirements.
>
> third party apps monkey patching libraries wreak havoc with testing local changes, etc, to those libraries.
>
> please do not do this.

While I agree with the general point about monkeypatches:

(a) this patch, which only involves changing a flag from True to
False, is only applied when running tests against a sqlite backend.  I
can't think of a scenario whereby this would wreak any havoc?

(b) putting up a patched tarball of sqlalchemy isn't painless either.
It creates a bit more cognitive-maintenance overhead for when we do
sqlalchemy updates (e.g. how will whoever does an update in a year
know why we have our own version? should we maintain the patch next to
it somewhere for that? etc)

The point in (b) isn't a big deal, but I maintain neither is the
potential issue with a monkeypatch, *in this specific case*.  Plus, I
haven't got time to think about it again until next week ;-)

Have a look at the patch
(https://bitbucket.org/sebbacon/ckan/changeset/ad9f51642166), and if
you're still convinced it might screw things up I'll happily change it
next week.

Thanks,

Seb