Re: [Catalyst] Re: Warnings when upgrading Catalyst
Hello,
Tomas Doran wrote:
On 29 Jun 2009, at 11:40, George Nistorica wrote:
Tomas Doran wrote:
Hmm, I can't replicate this in a simple case, from that description..
Here: http://www.depechemode.ro/MyApp-0.1.tar.gz
a simple TestApp using Catalyst::Controller::REST to replicate the
problem, having Catalyst 5.80005 installed.
Aha, this was very helpful, thank you.
The issue is inside Action::REST. Specifically, you could only trigger
it inside the 'action_GET' type methods.
I've had a fiddle with it so that the REST actions come up in stats, and
fixed the warning on the way:
http://github.com/bobtfish/catalyst-action-rest/commit/679978b1f8a1f309ff7c11ea0a744f8b1fe22d5a
This isn't ready to integrate yet, I think I probably need to rewrite it
all to be less fugly when I've got a clear head, but it does the job.
Can you check it out and confirm that this fixes the warnings for you?
I've checked it out and indeed using the version of Controller::REST you
provided fixes the warning.
Not only that, but it seems it doesn't break the functionality for both
the test application I provided and another more complex one I'm using.
I guess I should also go fix the warning in Catalyst, and get the stats
to display properly without this patch - but that is at best papering
over the cracks :)
:)
From your original mail, you said:
that worked for me, and now the debug output shows the profiling for the
methods I -forward() to as well:
the methods from another Controller to which I forward to, where missing
from the profiling output, getting that warning instead.
I can't see any methods missing from the stats table with / without this
patch when using your supplied testapp however.
Indeed, the test application provided didn't show the behaviour I was
talking about. my bad :)
Can you retry with the latest version of Catalyst so see if you can
replicate this behavior once again, before we kill the warning and
forget about this?
this replicates with Catalyst: 5.80005 Catalyst 5.80007
without the REST patch
the REST patch fixes the warning with both versions of Catalyst
Use of uninitialized value in concatenation (.) or string at
/usr/lib/perl5/site_perl/5.10.0/Catalyst.pm line 1573.
Cheers
t0m
___
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/
--
#!/usr/bin/perl -w ###
use strict;###
( $_ = qq{0912 3c1217 709073043p2it//,
'70kc1H 270P 70htonA t3uJ tni7p'8;})
=~tr/42179038/(larves)/;eval#uate;
##
___
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/
Re: [Catalyst] Re: Warnings when upgrading Catalyst
George Nistorica wrote: I've checked it out and indeed using the version of Controller::REST you provided fixes the warning. Not only that, but it seems it doesn't break the functionality for both the test application I provided and another more complex one I'm using. Great, thanks for testing. I'll see if I can make the code any less gross and/or come up with a neater way of doing it, then get it integrated and shipped. I guess I should also go fix the warning in Catalyst, and get the stats to display properly without this patch - but that is at best papering over the cracks :) :) From your original mail, you said: that worked for me, and now the debug output shows the profiling for the methods I -forward() to as well: the methods from another Controller to which I forward to, where missing from the profiling output, getting that warning instead. I can't see any methods missing from the stats table with / without this patch when using your supplied testapp however. Indeed, the test application provided didn't show the behaviour I was talking about. my bad :) heh, no worries. Can you retry with the latest version of Catalyst so see if you can replicate this behavior once again, before we kill the warning and forget about this? this replicates with Catalyst: 5.80005 Catalyst 5.80007 without the REST patch the REST patch fixes the warning with both versions of Catalyst Use of uninitialized value in concatenation (.) or string at /usr/lib/perl5/site_perl/5.10.0/Catalyst.pm line 1573. Nono - I was _specifically_ talking about the lines missing from the stats table behavior. I'm still concerned about working out the root cause of that, before I do something to 'just shut the warning up', as it were :) Cheers t0m ___ 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/
Re: [Catalyst] Re: Warnings when upgrading Catalyst
Tomas Doran wrote:
[...]
Can you retry with the latest version of Catalyst so see if you can
replicate this behavior once again, before we kill the warning and
forget about this?
this replicates with Catalyst: 5.80005 Catalyst 5.80007
without the REST patch
the REST patch fixes the warning with both versions of Catalyst
Use of uninitialized value in concatenation (.) or string at
/usr/lib/perl5/site_perl/5.10.0/Catalyst.pm line 1573.
Nono - I was _specifically_ talking about the lines missing from the
stats table behavior.
I'm still concerned about working out the root cause of that, before I
do something to 'just shut the warning up', as it were :)
Running $ perl script/myapp_test.pl /rest/test
in several scenarios:
* Catalyst 5.80007 from CPAN, Catalyst::Controller::REST 0.73 from CPAN
.+---.
| Action | Time |
++---+
| /rest/begin| 0.000156s |
| /rest/test | 0.000118s |
| - /test/private_forwarded_action | 0.000239s |
| /rest/end | 0.002551s |
'+---'
* Catalyst 5.80007, with warning supressed, Catalyst::Controller::REST
0.73 from CPAN
.+---.
| Action | Time |
++---+
| /rest/begin| 0.000176s |
| /rest/test | 0.000131s |
| - /test/private_forwarded_action | 0.000236s |
| /rest/end | 0.002756s |
'+---'
* Catalyst 5.80007 from CPAN, Catalyst::Controller::REST 0.73 you provided
.+---.
| Action | Time |
++---+
| /rest/begin| 0.000165s |
| /rest/test | 0.000140s |
| /rest/test_GET | 0.001048s |
| - /test/private_forwarded_action | 0.000259s |
| /rest/end | 0.004020s |
'+---'
* Catalyst 5.80007, with warning supressed, Catalyst::Controller::REST
0.73 you provided
.+---.
| Action | Time |
++---+
| /rest/begin| 0.000202s |
| /rest/test | 0.000128s |
| /rest/test_GET | 0.000937s |
| - /test/private_forwarded_action | 0.000260s |
| /rest/end | 0.003944s |
'+---'
It looks like I was mistaken when talking about missing actions in the
stats output, in the sense that supressing the Catalyst warning didn't
automagically make more actions to be displayed in the stats.
However I see that using the REST controller you provided, test_GET pops up.
Cheers
t0m
___
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/
--
#!/usr/bin/perl -w ###
use strict;###
( $_ = qq{0912 3c1217 709073043p2it//,
'70kc1H 270P 70htonA t3uJ tni7p'8;})
=~tr/42179038/(larves)/;eval#uate;
##
___
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/
Re: [Catalyst] Re: Warnings when upgrading Catalyst
- Original Message From: Tomas Doran bobtf...@bobtfish.net George Nistorica wrote: I've checked it out and indeed using the version of Controller::REST you provided fixes the warning. Not only that, but it seems it doesn't break the functionality for both the test application I provided and another more complex one I'm using. Great, thanks for testing. Thanks from me, too. This will make our test suite much quieter. Cheers, Ovid -- Buy the book - http://www.oreilly.com/catalog/perlhks/ Tech blog- http://use.perl.org/~Ovid/journal/ Twitter - http://twitter.com/OvidPerl Official Perl 6 Wiki - http://www.perlfoundation.org/perl6 ___ 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/
Re: [Catalyst] Re: Warnings when upgrading Catalyst
On 2 Jul 2009, at 12:55, George Nistorica wrote: It looks like I was mistaken when talking about missing actions in the stats output, in the sense that supressing the Catalyst warning didn't automagically make more actions to be displayed in the stats. Righto, glad that's cleared up. However I see that using the REST controller you provided, test_GET pops up. Yup. Getting that stuff in stats is also one of the benefits of that patch :) Thanks very much for all the effort testing various things out for me. I'll have a poke with Catalyst 5.71 some time over the weekend, work out why it didn't ever emit a warning, and work out the best way to get this fixed in core. I _MAY_ get time to get the patch on Action-REST into a shippable state, but that is I'm afraid less likely - it will happen, but probably not for a week or so. Cheers t0m ___ 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/
Re: [Catalyst] Re: Warnings when upgrading Catalyst
On 2 Jul 2009, at 14:03, Ovid wrote: Thanks from me, too. This will make our test suite much quieter. I guess this means that was also your issue, and I can stop worrying about that also then? ;_) Cheers t0m ___ 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/
Re: [Catalyst] Re: Warnings when upgrading Catalyst
Tomas Doran wrote:
On 26 Jun 2009, at 21:41, George Nistorica wrote:
Hi,
(my reply might be garbled, sorry)
Makes perfect sense here.
my email was garbled as I wasn't subscribed to the list when the
original emails have been sent :P
I've got the same problem generated from a Controller that uses
Controller::REST as a base class.
And the warning being generated when I -forward() to another Controller
(doing some data validation).
Hmm, I can't replicate this in a simple case, from that description..
Here: http://www.depechemode.ro/MyApp-0.1.tar.gz
a simple TestApp using Catalyst::Controller::REST to replicate the
problem, having Catalyst 5.80005 installed.
Please have a look at the included README.
I've got around this warning by editing Catalyst.pm and adding this
piece of code
if ( not exists $c-counter-{$parent}
or not defined $c-counter-{$parent} )
{
$c-counter-{$parent} = 0;
}
right before the undefined value is used:
$c-stats-profile(
begin = $action,
parent = $parent . $c-counter-{$parent},
uid= $uid,
);
that worked for me, and now the debug output shows the profiling for the
methods I -forward() to as well:
the methods from another Controller to which I forward to, where missing
from the profiling output, getting that warning instead.
That looks like a prefect fix to me, but I'd _really_ like to get to the
bottom of what is causing the issue, so that we can write tests of some
form, and also because this could be hiding a bug at another layer which
should be fixed :/
that makes sense, what I did was just to shut off the warning, didn't
had the guts to look forward to search for the root of the problem.
Are you forwarding to a private path string, or an action object?
I guess extracting _just_ the structure of your controllers (without any
of the code, just the sub/attribute declerations), and ensuring your
doing the appropriate forwards that your app would be doing would
replicate the issue in a TestApp.
Don't suppose you could attempt that, so that we can find the root
cause, and get this fix integrated?
Cheers
t0m
___
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/
--
#!/usr/bin/perl -w ###
use strict;###
( $_ = qq{0912 3c1217 709073043p2it//,
'70kc1H 270P 70htonA t3uJ tni7p'8;})
=~tr/42179038/(larves)/;eval#uate;
##
___
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/
Re: [Catalyst] Re: Warnings when upgrading Catalyst
On 26 Jun 2009, at 21:41, George Nistorica wrote:
Hi,
(my reply might be garbled, sorry)
Makes perfect sense here.
I've got the same problem generated from a Controller that uses
Controller::REST as a base class.
And the warning being generated when I -forward() to another
Controller
(doing some data validation).
Hmm, I can't replicate this in a simple case, from that description..
I've got around this warning by editing Catalyst.pm and adding this
piece of code
if ( not exists $c-counter-{$parent}
or not defined $c-counter-{$parent} )
{
$c-counter-{$parent} = 0;
}
right before the undefined value is used:
$c-stats-profile(
begin = $action,
parent = $parent . $c-counter-{$parent},
uid= $uid,
);
that worked for me, and now the debug output shows the profiling
for the
methods I -forward() to as well:
the methods from another Controller to which I forward to, where
missing
from the profiling output, getting that warning instead.
That looks like a prefect fix to me, but I'd _really_ like to get to
the bottom of what is causing the issue, so that we can write tests
of some form, and also because this could be hiding a bug at another
layer which should be fixed :/
Are you forwarding to a private path string, or an action object?
I guess extracting _just_ the structure of your controllers (without
any of the code, just the sub/attribute declerations), and ensuring
your doing the appropriate forwards that your app would be doing
would replicate the issue in a TestApp.
Don't suppose you could attempt that, so that we can find the root
cause, and get this fix integrated?
Cheers
t0m
___
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/
[Catalyst] Re: Warnings when upgrading Catalyst
Hi,
(my reply might be garbled, sorry)
I've got the same problem generated from a Controller that uses
Controller::REST as a base class.
And the warning being generated when I -forward() to another Controller
(doing some data validation).
I've got around this warning by editing Catalyst.pm and adding this
piece of code
if ( not exists $c-counter-{$parent}
or not defined $c-counter-{$parent} )
{
$c-counter-{$parent} = 0;
}
right before the undefined value is used:
$c-stats-profile(
begin = $action,
parent = $parent . $c-counter-{$parent},
uid= $uid,
);
that worked for me, and now the debug output shows the profiling for the
methods I -forward() to as well:
the methods from another Controller to which I forward to, where missing
from the profiling output, getting that warning instead.
Gah! I should really consider just giving up on Yahoo! Mail. Replying
to HTML email is *painful* :( I hereby swallow the meme kool-aid and
dub them Yahoo! FAIL.
--- On Wed, 24/6/09, Tomas Doran bobtfish[at]bobtfish.net wrote:
From: Tomas Doran bobtfish[at]bobtfish.net Also, you're in debug
mode.. Do you get the warnings when debug mode is disabled?
Cool! When I disable debug mode, the warnings go away. Never thought
to look at that. This makes me feel much more confident that we don't
have a bug in our code. Still, we run tests in debug mode and we don't
want our test suite spewing warnings lest we start ignoring them.
return if ( ( $code-name =~ /^_.*/ )
(!$c-config-{show_internal_actions} )); So I guess you have
$c-config-{show_internal_actions} turned on then?
No, that evaluates to undef. However, that doesn't apply because when
the error occurs, $code-name is 'default', thus avoiding the second
condition. I think the confusion stems from how the above code is
written. This is clearer, I think:
return if ( $code-name =~ /^_/ and not
$c-config-{show_internal_actions} );
The $code-name when we get the warning is 'default', so I'm
assuming it's not considered a private action. When we get to the
actual section of code which issues the warning (line 1561 in the
cpan distribution) I'm now wildly stabbing in the dark, but can you
try:
http://dev.catalyst.perl.org/repos/Catalyst/Catalyst-Runtime/5.80/branches/index_default_fuckage/
which _may_ fix your issue, as the sub you're dealing with is called
'default' and that has some fixes for that... This is merely a
guess, I don't pretend to really understand what's going on for you,
or why you're getting this.
I tried to grab that, but our socks proxy won't allow this. If anyone
is kind enough to send me a tarball, I'll happily give it a try and
report what I find :)
And no worries about stabs in the dark. I really appreciate the fact
that you're taking the time to consider this.
Cheers, Ovid -- Buy the book - http://www.oreilly.com/catalog/perlhks/
Tech blog - http://use.perl.org/~Ovid/journal/ Twitter -
http://twitter.com/OvidPerl Official Perl 6 Wiki -
http://www.perlfoundation.org/perl6
___ List:
Catalyst[at]lists.scsys.co.uk Listinfo:
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable
archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.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/
