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

History Show all quoted text
  Tue Jul 17 17:25:37 2012 mods [...] hank.org - Ticket created
Subject: Replicated doesn't support accessor connect_info.
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,
  Sat Nov 03 13:51:49 2012 ribasushi [...] leporine.io - Cc JJNAPIORK added
  Sat Nov 03 13:54:15 2012 ribasushi [...] leporine.io - Correspondence added
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.
  Sat Nov 03 13:54:16 2012 The RT System itself - Status changed from 'new' to 'open'
  Sat Nov 03 15:21:09 2012 mods [...] hank.org - Correspondence added
RT-Send-CC: jjnapiork [...] cpan.org
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.
  Sat Nov 03 15:28:24 2012 ribasushi [...] leporine.io - Correspondence added
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.
  Sun Nov 18 18:11:47 2012 ribasushi [...] leporine.io - Correspondence added
RT-Send-CC: jjnapiork [...] cpan.org
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?
  Mon Nov 26 11:54:49 2012 jjnapiork [...] cpan.org - Correspondence added
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
  Mon Nov 26 12:03:37 2012 moseley [...] hank.org - Correspondence added
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
  Tue Dec 11 06:36:10 2012 ribasushi [...] leporine.io - Correspondence added
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
  Tue Dec 11 06:36:13 2012 ribasushi [...] leporine.io - Status changed from 'open' to 'patched'
  Wed Jan 23 07:00:21 2013 ribasushi [...] leporine.io - Status changed from 'patched' to 'resolved'
  Wed Jan 23 07:00:22 2013 ribasushi [...] leporine.io - Fixed in 0.08205 added