pplu / aws-sdk-perl

A community AWS SDK for Perl Programmers
Other
170 stars 94 forks source link

ML: TrainingParameters are not parsed correctly #45

Closed dtikhonov closed 9 years ago

dtikhonov commented 9 years ago

I discovered that Paws::API::Caller does not parse out TrainingParameters correctly.

I use the following script to list my MLModels:

use Paws;
Paws->default_config->caller('Paws::Net::LWPCaller');

my $ml = Paws->service('MachineLearning', region => 'us-east-1');

my $models = $ml->DescribeMLModels->Results;
for my $m (@$models) {
    print $m->MLModelId, " -> ", $m->Name, "\n";
}

Which would fail with an error like this:

Use of uninitialized value in anonymous hash ({}) at /home/dmitri/devel/pplu-aws-sdk-perl/lib/Paws/API/Caller.pm line 198.
Attribute (Map) does not pass the type constraint because: Validation failed for 'HashRef[Str]' with value HASH(0x48379b0) at /home/dmitri/.plenv/versions/5.18.2/lib/perl5/site_perl/5.18.2/x86_64-linux/Moose/Object.pm line 24
        Moose::Object::new('Paws::MachineLearning::TrainingParameters', 'Map', 'HASH(0x48379b0)') called at /home/dmitri/devel/pplu-aws-sdk-perl/lib/Paws/API/Caller.pm line 195
        Paws::API::Caller::new_from_struct('Paws::MachineLearning=HASH(0x3416d48)', 'Paws::MachineLearning::MLModel', 'HASH(0x44731d0)') called at /home/dmitri/devel/pplu-aws-sdk-perl/lib/Paws/API/Caller.pm line 264
        Paws::API::Caller::new_from_struct('Paws::MachineLearning=HASH(0x3416d48)', 'Paws::MachineLearning::DescribeMLModelsOutput', 'HASH(0x4021ff0)') called at /home/dmitri/devel/pplu-aws-sdk-perl/lib/Paws/API/Caller.pm line 116
        Paws::API::Caller::response_to_object('Paws::MachineLearning=HASH(0x3416d48)', 'HASH(0x4021ff0)', 'Paws::MachineLearning::DescribeMLModels=HASH(0x3e6ee90)', 200, '{"Results":[....
....

$value looks like this:

$VAR1 = {
          'sgd.maxPasses' => '10',
          'sgd.l2RegularizationAmount' => '1e-6',
          'sgd.l1RegularizationAmount' => '0.0',
          'sgd.maxMLModelSizeInBytes' => '104857600',
          'algorithm' => 'sgd'
        };

I was able to work around the problem using the following modification, but obviously there must be a right way to fix it:

diff --git a/lib/Paws/API/Caller.pm b/lib/Paws/API/Caller.pm
index 7fed504..c14a1b2 100644
--- a/lib/Paws/API/Caller.pm
+++ b/lib/Paws/API/Caller.pm
@@ -179,6 +179,7 @@ package Paws::API::Caller {
                 my $xml_keys = $att_class->xml_keys;
                 my $xml_values = $att_class->xml_values;

+                my $workaround;
                 if ($value_ref eq 'HASH') {
                   if (exists $value->{ member }) {
                     $value = $value->{ member };
@@ -187,12 +188,15 @@ package Paws::API::Caller {
                   } elsif (keys %$value == 1) {
                     $value = $value->{ (keys %$value)[0] };
                   } else {
+                    $workaround = 1;
                     #die "Can't detect the item that has the array in the response hash";
                   }
                   $value_ref = ref($value);
                 }

-                if ($value_ref eq 'ARRAY') {
+                if ($workaround) {
+                  $args{ $att } = $att_class->new(Map => $value);
+                } elsif ($value_ref eq 'ARRAY') {
                   $args{ $att } = $att_class->new(Map => { map { ( $_->{ $xml_keys } => $_->{ $xml_values } ) } @$value } );
                 } elsif ($value_ref eq 'HASH') {
                   $args{ $att } = $att_class->new(Map => { $value->{ $xml_keys } => $value->{ $xml_values } } );
pplu commented 9 years ago

Hi!

It'd be nice if you can dump the body of the response into t/10_responses/. Take a look at t/10_responses/cloudwatch-describe-alarm-history.response and t/10_responses/cloudwatch-describe-alarm-history.response.test.yml, for example. This will help me reproduce the bug without needing to call. It's quite easy to create a test case with t/lib/TestMakerCaller.pm. I suspect you have to make a separate one for LWP, but it's basically inheriting from LWPCaller instead.

Feel free to dump a bunch of calls for any service you like. The more tests, the better the SDK :)

Please remember to change any data in the test cases you feel is private/non-disclosable.

pplu commented 9 years ago

Paws 0.14 is on CPAN with the fix for this issue. Cheers! (and thanks!)