Mailing List Archive: Bump Catalyst::Model::Adaptor

Bump Catalyst::Model::Adaptor

Dec 15, 2013, 8:27 AM

Post #1 of 5 (2604 views)

I'm a big fan of Catalyst::Model::Adaptor. It helps maintain good
separation of concerns and code resue.

But, there's an annoying issue where it sends its entire config to the
model's constructor instead of just what is in "args".

Some code will complain when it finds unknown constructor arguments. For
example I'm seeing lots of these:

Unrecognised options: args catalyst_component_name class at
/var/lib/jenkins/perl5/perlbrew/perls/jenkins/lib/site_perl/5.14.4/Catalyst/Model/Adaptor/Base.pm
line 27.

In this case it is Cache::Memcached::libmemcached complaining.

I think there's a suggested patch in one of these. Is there someone that
can review and maybe push out a new version?

https://rt.cpan.org/Public/Bug/Display.html?id=67078
https://rt.cpan.org/Public/Bug/Display.html?id=78663

--
Bill Moseley
moseley@hank.org

Re: Bump Catalyst::Model::Adaptor [ In reply to ]

neil at mylunn

Dec 15, 2013, 4:49 PM

Post #2 of 5 (2533 views)

Permalink

On 16/12/2013 3:27 AM, Bill Moseley wrote:
> I'm a big fan of Catalyst::Model::Adaptor. It helps maintain good
> separation of concerns and code resue.
>
> But, there's an annoying issue where it sends its entire config to the
> model's constructor instead of just what is in "args".
>
> Some code will complain when it finds unknown constructor arguments.
> For example I'm seeing lots of these:
>
> Unrecognised options: args catalyst_component_name class at
> /var/lib/jenkins/perl5/perlbrew/perls/jenkins/lib/site_perl/5.14.4/Catalyst/Model/Adaptor/Base.pm
> line 27.
Hi Bill,

Isn't this just a case of adding to your model class:

sub mangle_arguments {
my ( $self, $args ) = @_;
return $args->{args};
}

Thus overriding the default with what you want to do for your constructor.
This clears the error for me.

>
> In this case it is Cache::Memcached::libmemcached complaining.
>
> I think there's a suggested patch in one of these. Is there someone
> that can review and maybe push out a new version?
>
> https://rt.cpan.org/Public/Bug/Display.html?id=67078
> https://rt.cpan.org/Public/Bug/Display.html?id=78663
>
>
>
> --
> Bill Moseley
> moseley@hank.org <mailto:moseley@hank.org>
>
>
> _______________________________________________
> List: Catalyst@lists.scsys.co.uk
> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
> Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
> Dev site: http://dev.catalyst.perl.org/

---
This email is free from viruses and malware because avast! Antivirus protection is active.
http://www.avast.com

Re: Bump Catalyst::Model::Adaptor [ In reply to ]

moseley at hank

Dec 15, 2013, 5:53 PM

Post #3 of 5 (2529 views)

Permalink

On Sun, Dec 15, 2013 at 4:49 PM, neil.lunn <neil@mylunn.id.au> wrote:

>
> Isn't this just a case of adding to your model class:
>

Yes, that's good. Thanks.

>
> sub mangle_arguments {
> my ( $self, $args ) = @_;
> return $args->{args};
> }
>
> Thus overriding the default with what you want to do for your constructor.
> This clears the error for me.
>
>
> In this case it is Cache::Memcached::libmemcached complaining.
>
> I think there's a suggested patch in one of these. Is there someone
> that can review and maybe push out a new version?
>
> https://rt.cpan.org/Public/Bug/Display.html?id=67078
> https://rt.cpan.org/Public/Bug/Display.html?id=78663
>
>
>
> --
> Bill Moseley
> moseley@hank.org
>
>
> _______________________________________________
> List: Catalyst@lists.scsys.co.uk
> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
> Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
> Dev site: http://dev.catalyst.perl.org/
>
>
>
>
> ------------------------------
> <http://www.avast.com/>
>
> This email is free from viruses and malware because avast! Antivirus<http://www.avast.com/>protection is active.
>
>
> _______________________________________________
> List: Catalyst@lists.scsys.co.uk
> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
> Searchable archive:
> http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
> Dev site: http://dev.catalyst.perl.org/
>
>

--
Bill Moseley
moseley@hank.org

Re: Bump Catalyst::Model::Adaptor [ In reply to ]

neil at mylunn

Dec 15, 2013, 6:09 PM

Post #4 of 5 (2526 views)

Permalink

On 16/12/2013 12:53 PM, Bill Moseley wrote:
>
>
>
> On Sun, Dec 15, 2013 at 4:49 PM, neil.lunn <neil@mylunn.id.au
> <mailto:neil@mylunn.id.au>> wrote:
>
>
> Isn't this just a case of adding to your model class:
>
>
> Yes, that's good. Thanks.
>
>
> sub mangle_arguments {
> my ( $self, $args ) = @_;
> return $args->{args};
> }
>

An alternate is overriding prepare_arguments as in the previous release:
http://cpansearch.perl.org/src/BOBTFISH/Catalyst-Model-Adaptor-0.09/lib/Catalyst/Model/Adaptor/Base.pm

But I prefer the mangle_arguments particulary for classes that do not
use a Moose like contructor.
The change from the old code seems to be deliberate in passing in extra
arguments to the model constructor, so it seems best to override these
hooks where needed rather than patch the base.

>
> Thus overriding the default with what you want to do for your
> constructor.
> This clears the error for me.
>
>>
>> In this case it is Cache::Memcached::libmemcached complaining.
>>
>> I think there's a suggested patch in one of these. Is there
>> someone that can review and maybe push out a new version?
>>
>> https://rt.cpan.org/Public/Bug/Display.html?id=67078
>> https://rt.cpan.org/Public/Bug/Display.html?id=78663
>>
>>
>>
>> --
>> Bill Moseley
>> moseley@hank.org <mailto:moseley@hank.org>
>>
>>
>> _______________________________________________
>> List:Catalyst@lists.scsys.co.uk <mailto:Catalyst@lists.scsys.co.uk>
>> Listinfo:http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
>> Searchable archive:http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
>> Dev site:http://dev.catalyst.perl.org/
>
>
>
> ------------------------------------------------------------------------
> <http://www.avast.com/>
>
> This email is free from viruses and malware because avast!
> Antivirus <http://www.avast.com/> protection is active.
>
>
>
> _______________________________________________
> List: Catalyst@lists.scsys.co.uk <mailto:Catalyst@lists.scsys.co.uk>
> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
> Searchable archive:
> http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
> Dev site: http://dev.catalyst.perl.org/
>
>
>
>
> --
> Bill Moseley
> moseley@hank.org <mailto:moseley@hank.org>
>
>
> _______________________________________________
> List: Catalyst@lists.scsys.co.uk
> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
> Searchable archive: http://www.mail-archive.com/catalyst@lists.scsys.co.uk/
> Dev site: http://dev.catalyst.perl.org/

---
This email is free from viruses and malware because avast! Antivirus protection is active.
http://www.avast.com

Re: Bump Catalyst::Model::Adaptor [ In reply to ]

moseley at hank

Dec 15, 2013, 11:20 PM

Post #5 of 5 (2528 views)

Permalink

On Sun, Dec 15, 2013 at 6:09 PM, neil.lunn <neil@mylunn.id.au> wrote:

>
>
> But I prefer the mangle_arguments particulary for classes that do not use
> a Moose like contructor.
> The change from the old code seems to be deliberate in passing in extra
> arguments to the model constructor, so it seems best to override these
> hooks where needed rather than patch the base.
>

It's a bit confusing, though. The docs for "args" say this:

This is a hashref of arguments to pass to the constructor of class. It is
optional, of course. If you omit it, nothing is passed to the constructor
(as opposed to {}, an empty hashref).

But maybe there was a use case where having the extra config (in addition
to config in "args") passed to the constructor was necessary.

--
Bill Moseley
moseley@hank.org