Skip Menu |
 

This queue is for tickets about the DBIx-Class CPAN distribution.

Report information
The Basics
Id: 78436
Status: resolved
Priority: 0/
Queue: DBIx-Class

People
Owner: Nobody in particular
Requestors: mods [...] hank.org
Cc: jjnapiork [...] cpan.org
AdminCc:

Bug Information
Severity: Normal
Broken in: 0.08198
Fixed in: 0.08205



Subject: Replicated doesn't support accessor connect_info.
Download (untitled) / with headers
text/plain 557b
DBIx::Class::Storage::DBI has this, which returns connect info if not passed an argument. sub connect_info { my ($self, $info) = @_; return $self->_connect_info if !$info; It's not clear that is documented behavior, but it does act as an accessor. But DBIx::Class::Storage::DBI::Replicated wraps connect_info and assumes that connect info is always passed in (and resets connect info if called to read connect_info). The behavior should be the same with and without replication enabled (regardless of which usage is correct). Thanks, Thanks,
Download (untitled) / with headers
text/plain 1.1k
On Tue Jul 17 17:25:37 2012, HANK wrote: Show quoted text
> DBIx::Class::Storage::DBI has this, which returns connect info if not > passed an argument. > > sub connect_info { > my ($self, $info) = @_; > > return $self->_connect_info if !$info; > > It's not clear that is documented behavior, but it does act as an > accessor. > > But DBIx::Class::Storage::DBI::Replicated wraps connect_info and > assumes that connect info is > always passed in (and resets connect info if called to read > connect_info). > > The behavior should be the same with and without replication enabled > (regardless of which > usage is correct).
It can not be the same. While connect_info is a classic accessor (getter/setter), the connect_info of *each* member of the pool is obviously different. Hence it can not return any single value. I am not the original designer of this code, and hence I can't really say what is the correct approach (I only know it can not 100% mimic the core method's behavior). The easiest thing I could think of to at least prevent "shooting in the foot" like you experience is this: https://github.com/dbsrgits/dbix-class/commit/8ff8a120d. Let me know what you think. I am also adding John on CC, so he could chime in.
RT-Send-CC: jjnapiork [...] cpan.org
Download (untitled) / with headers
text/plain 1.6k
On Sat Nov 03 13:54:15 2012, RIBASUSHI wrote: Show quoted text
> On Tue Jul 17 17:25:37 2012, HANK wrote:
> > DBIx::Class::Storage::DBI has this, which returns connect info if not > > passed an argument. > > > > sub connect_info { > > my ($self, $info) = @_; > > > > return $self->_connect_info if !$info; > > > > It's not clear that is documented behavior, but it does act as an > > accessor. > > > > But DBIx::Class::Storage::DBI::Replicated wraps connect_info and > > assumes that connect info is > > always passed in (and resets connect info if called to read > > connect_info). > > > > The behavior should be the same with and without replication enabled > > (regardless of which > > usage is correct).
> > It can not be the same. While connect_info is a classic accessor > (getter/setter), the connect_info of *each* member of the pool is > obviously different. Hence it can not return any single value. > > I am not the original designer of this code, and hence I can't really > say what is the correct approach (I only know it can not 100% mimic the > core method's behavior). The easiest thing I could think of to at least > prevent "shooting in the foot" like you experience is this: > https://github.com/dbsrgits/dbix-class/commit/8ff8a120d. Let me know > what you think. > > I am also adding John on CC, so he could chime in.
I think that's fine. I need to rethink my problem anyway. I have a table with a time_zone column that (sadly) can be NULL. So, the hack here was in the Result class to be able to inspect the connect_info for an extra default_time_zone config. A better plan might be an extra attribute in the Schema class. Or just fix the database.
Download (untitled) / with headers
text/plain 370b
On Sat Nov 03 15:21:09 2012, HANK wrote: Show quoted text
> > I have a table with a time_zone column that (sadly) can be NULL. So, > the hack here was in the > Result class to be able to inspect the connect_info for an extra > default_time_zone config.
Note - $schema->storage->master->connect_info ought to work just like you would expect. Please let me know if this is not the case.
RT-Send-CC: jjnapiork [...] cpan.org
Download (untitled) / with headers
text/plain 823b
On Sat Nov 03 13:54:15 2012, RIBASUSHI wrote: Show quoted text
> On Tue Jul 17 17:25:37 2012, HANK wrote:
> > The behavior should be the same with and without replication enabled > > (regardless of which > > usage is correct).
> > It can not be the same. While connect_info is a classic accessor > (getter/setter), the connect_info of *each* member of the pool is > obviously different. Hence it can not return any single value. > > I am not the original designer of this code, and hence I can't really > say what is the correct approach (I only know it can not 100% mimic the > core method's behavior). The easiest thing I could think of to at least > prevent "shooting in the foot" like you experience is this: > https://github.com/dbsrgits/dbix-class/commit/8ff8a120d. Let me know > what you think. >
John, any word/ideas on this?
Download (untitled) / with headers
text/plain 1.2k
On Sun Nov 18 18:11:47 2012, RIBASUSHI wrote: Show quoted text
> On Sat Nov 03 13:54:15 2012, RIBASUSHI wrote:
> > On Tue Jul 17 17:25:37 2012, HANK wrote:
> > > The behavior should be the same with and without replication enabled > > > (regardless of which > > > usage is correct).
> > > > It can not be the same. While connect_info is a classic accessor > > (getter/setter), the connect_info of *each* member of the pool is > > obviously different. Hence it can not return any single value. > > > > I am not the original designer of this code, and hence I can't really > > say what is the correct approach (I only know it can not 100% mimic the > > core method's behavior). The easiest thing I could think of to at least > > prevent "shooting in the foot" like you experience is this: > > https://github.com/dbsrgits/dbix-class/commit/8ff8a120d. Let me know > > what you think. > >
> > John, any word/ideas on this? >
Hey Sorry for the slow response. I guess if the compatible behavior is for connect-info to work like an accessor I think that would be best. My guess here is that ideally storage->connect_info without args work return the master connect info, and maybe the replicants under a private key as an array. Thoughts? If someone updates the replication test case I can make it pass. John
Subject: Re: [rt.cpan.org #78436] Replicated doesn't support accessor connect_info.
Date: Mon, 26 Nov 2012 09:02:49 -0800
To: bug-DBIx-Class [...] rt.cpan.org
From: Bill Moseley <moseley [...] hank.org>
On Mon, Nov 26, 2012 at 8:54 AM, John Napiorkowski via RT < bug-DBIx-Class@rt.cpan.org> wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=78436 > > > On Sun Nov 18 18:11:47 2012, RIBASUSHI wrote:
> > On Sat Nov 03 13:54:15 2012, RIBASUSHI wrote:
> > > On Tue Jul 17 17:25:37 2012, HANK wrote:
> > > > The behavior should be the same with and without replication enabled > > > > (regardless of which > > > > usage is correct).
> > > > > > It can not be the same. While connect_info is a classic accessor > > > (getter/setter), the connect_info of *each* member of the pool is > > > obviously different. Hence it can not return any single value. > > > > > > I am not the original designer of this code, and hence I can't really > > > say what is the correct approach (I only know it can not 100% mimic the > > > core method's behavior). The easiest thing I could think of to at least > > > prevent "shooting in the foot" like you experience is this: > > > https://github.com/dbsrgits/dbix-class/commit/8ff8a120d. Let me know > > > what you think. > > >
> > > > John, any word/ideas on this? > >
> > Hey > > Sorry for the slow response. I guess if the compatible behavior is for > connect-info to work like > an accessor I think that would be best. My guess here is that ideally > storage->connect_info > without args work return the master connect info, and maybe the replicants > under a private key > as an array. Thoughts? >
First, I'm not sure this is a very important issue. I was trying to hide some configuration info in connect_info so that a result class could use it. It was a bad idea. We have servers in different countries and we had the need to set a default timezone based on some config setting and connect_info was a handy, common place to attempt that. Not a good approach. (Having a NULLable time_zone column is our real problem...) It would be nice if connect_info was consistent if it's going to be an accessor. But maybe the solution is simply documenting what connect_info returns. -- Bill Moseley moseley@hank.org
Download (untitled) / with headers
text/plain 323b
Show quoted text
> It would be nice if connect_info was consistent if it's going to be an > accessor. But maybe the solution is simply documenting what > connect_info > returns. >
I went for the exception as proposed before. This will at least not result in surprises down the road. We can always make it do something else. 1fccee7b0


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.