Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testcase to demonstrate that DBI is not always loaded before being used #23

Closed
wants to merge 1 commit into from

Conversation

willert
Copy link
Contributor

@willert willert commented Apr 6, 2013

A simple 'use DBI;' in DBIx::Class::Storage::DBI will fix this. The fix
is not included.

This is a somewhat esoteric bug, but it causes the tests of HTML::FormHandler::Model::DBIC to fail.

A simple 'use DBI;' in DBIx::Class::Storage::DBI will fix this. The fix
is not included.
@ribasushi
Copy link
Collaborator

I am an idiot - I knew the problem is there (as evidenced by the commit msg of 524343a) - I was just too dumb to realize the stable codebase can trigger it as well.

The correct fix (loading DBI.pm is not it, see commit msg above) is already contained in the 0.08250-to-be branch (https://github.com/dbsrgits/dbix-class/tree/topic/constructor_rewrite). It is currently waiting on one last wrinkle (https://twitter.com/AFresh1/status/320056535147499521) and then will sail to CPAN within a week (or we will have to ship a 0.08211 to fix the screwup in 0.08210)

Please do not adjust the HFH::M::DBIC tests - they are valid.Could you also test HFH against
ac09a390ca to make sure everything works as intended?

Thanks!

@willert
Copy link
Contributor Author

willert commented Apr 6, 2013

On Sat, Apr 6, 2013 at 8:43 PM, Peter Rabbitson notifications@github.comwrote:

The correct fix (loading DBI.pm is not it, see commit msg above) is
already contained in the 0.08250-to-be branch (
https://github.com/dbsrgits/dbix-class/tree/topic/constructor_rewrite).

I figured the fix would be more involved than that, that's why I didn't
include any try of my own to fix it.

Please do not adjust the HFH::M::DBIC tests - they are valid.Could you
also test HFH against ac09a39ac09a390cato make sure everything works as intended?

After cherry-picking my test into this commit no more errors are thrown.
The test does exactly what HFH::m::DBIC does (it's modeled after the
failure there, after all) so everything seems to work fine. If you need
further testing I might be able to to that tomorrow.

Cheers,
Sebastian

@ribasushi
Copy link
Collaborator

The fix isn't really "involved", it amounts more or less to this (read comment in linked hunk): http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits/DBIx-Class.git;a=commitdiff;h=524343a94#patch5

The rationale is that we want to delay loading of the DBI/DBD combo until a connection (or something that will result in a connection) is about to fire. It sounds a tad counterintuitive in the context of DBIC, but is a pretty reasonable expectation within the scope of a larger project.

As such I will close this ticket without merging the test (there is a test which failure already being triggered in the 0.08250 branch, as indicated in the linked merge). The HFH issue will be resolved by a new release by the end of next week.

Sorry for the trouble :(

@ribasushi ribasushi closed this Apr 6, 2013
@ribasushi
Copy link
Collaborator

Please test with DBIx::Class 0.08249_02 - it contains all the fixes dealing with this: https://metacpan.org/release/RIBASUSHI/DBIx-Class-0.08249_02

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants