On Sun, Sep 26, 2010 at 8:44 PM, Russell Keith-Magee
<russ...@keith-magee.com> wrote:
> On Mon, Sep 27, 2010 at 6:24 AM, Karen Tracey <kmtra...@gmail.com> wrote:
>> I've run it on Python 2.4 & 2.5 (Ubuntu, sqlite DB) with no problems.
>>
>> I do have some feedback on the @skipIfDB addition: I'd really like if this
>> could be used to distinguish between the different MySQL storage backends.
>> From a very brief look it seems like currently there are a bunch of tests
>> skipped when the DB backend is MySQL, under the assumption that MySQL does
>> not have transaction support. However MySQL does have transaction support
>> when the InnoDB storage engine is used, so it would be nice if these tests
>> were only skipped when the MySQL/MyISAM combination were in use, not when
>> MySQL/InnoDB was used.
>>
>> Similarly there are a bunch of tests that fail or produce errors when the
>> MySQL/InnoDB combination is in use, because that combination does not
>> support forward foreign-key references when loading fixtures. It's not ideal
>> to skip a bunch of tests for function that really should be tested on this
>> combination (aggregates, for example), but for now at least the fixtures
>> used by these tests do not allow them to pass on MySQL/InnoDB, so they might
>> as well just be skipped. I have quit ever trying to run the full Django test
>> suite using MySQL/InnoDB because there are just too many "known failures"
>> there. It would be nice if the addition of @skipIfDB improved that. (For
>> that matter I also never run the full suite with MySQL/MyISAM because the
>> lack of transaction support with that combo makes it just too slow to run
>> the full suite, but I don't know of any way to improve that situation.)
>>
>> One difficulty in adding this extra check is that it is hard to know for
>> sure what storage engine is in use. If the database definition dict includes
>> 'OPTIONS': { "init_command": "SET storage_engine=<whatever>" } then we know
>> the <whatever> engine is in use. If not, the default configured engine for
>> the MySQL server will be used, and that could be either one. I'd be inclined
>> to say if you want to skip tests based on a particular storage engine being
>> used, then you need to make it explicit in the test settings what engine you
>> are using. So I'd be happy if this @skipIfDB threw an error if it was asked
>> to skip based on storage engine but could not find in the test settings what
>> storage engine was being used.
>
> The problem with this approach is that you can configure MySQL to use
> InnoDB by default for table creation, so there won't necessarily be
> any Django settings providing evidence that transactions are
> available.
>
>> Another difficulty is figuring out how to nicely specify this additional
>> requirement. Have not given that a whole lot of thought yet. Right now the
>> @skipIfDB takes a simple string (matched against DB ENGINE key) or iterable
>> of strings. Supporting querying both ENGINE and OPTIONS, and allowing for
>> more than one to be listed, might get a bit ugly. I am curious why the
>> iterable option here was added -- are there a lot of cases in the current
>> suite where tests need to be skipped on a set of DB backends and not just
>> one?
>
> The use case for the iterable is everywhere that PostgreSQL is the
> skip target, because we have 2 backends that need to be skipped.
>
> I'm in agreement that the ENGINE-string based skipIfDB isn't a
> particularly robust test. If nothing else, it means that if a user
> writes a third-party engine for MySQL (or anything else) in order to
> change some subtle behavior, the test suite won't be any use to them,
> because the string match won't cause the appropriate tests to be
> skipped.
>
> One thought here would be to introduce a variable on the backend
> itself that is a 'db identifier' - i.e., a string that clearly
> identifies the vendor, independent of the path that has been used to
> install the backend. So, for example, a check for
> connection.db_identifier = 'postgresql' would always tell you that
> you're dealing with PostgreSQL, regardless of the import path to the
> actual backend.
>
> This has three potential benefits.
>
> 1. We can avoid the need for the iterable skipIfDB version.
> 2. We enable writers of third-party backend for Django supported
> databases to benefit from test skip annotations
> 3. We provide a name that can be used for backend-specific features,
> such as the index-hinting proposal that has been floating around.
>
> However, this still doesn't address the InnoDB problem, or provide any
> support for completely external backends -- for example, the writers
> of the DB2 backend can't use Django's test suite, because the DB2
> backend isn't specifically named in Django's source code (nor should
> it).
>
> It also occurs to me that in most cases, the skip should actually be
> based on capabilities, not on the backend itself. We should be
> skipping because the backend doesn't support transactions, or because
> the backend doesn't allow forward references in fixtures, not because
> the backend is MySQL.
>
> connection.features already provides one place to check whether
> certain features are available. It would be a relatively simple change
> to modify the test skips to check connection.features, rather than the
> ENGINE. This would have the added benefit that writers of backends
> that are not officially supported (e.g., DB2) would be able to skip
> the appropriate tests based purely upon the capabilities of their
> backend. Looking forward, this will be especially important when we
> get to supporting NoSQL backends.
>
> There are a couple of places that are literally "check that Oracle
> does this right, in an Oracle specific way", but they're the
> exception, not the rule.
>
> Opinions?
>
> Yours,
> Russ Magee %-)
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To post to this group, send email to django-develop...@googlegroups.com.
> To unsubscribe from this group, send email to
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>
+1 for using connection.features to do this stuff properly.
Alex
--
"I disapprove of what you say, but I will defend to the death your
right to say it." -- Voltaire
"The people's good is the highest law." -- Cicero
"Code can always be simpler than you think, but never as simple as you
want" -- Me
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
