pplu / aws-sdk-perl

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

Full Glacier support #30

Open pplu opened 9 years ago

pplu commented 9 years ago
carton exec perl -I lib/ -I auto-lib/ -I t/lib/ bin/paws Glacier --region eu-west-1 ListVaults accountId x
Paws::Glacier is not stable / supported / entirely developed at auto-lib/Paws/Glacier.pm line 2.
Required parameter missing: API version

Have to see how the version param is passed to the API

jvolkening commented 6 years ago

This is an old issue, but I am still getting the Required parameter missing: API version exception when trying to use Glacier. I can see in the codebase where the version is defined correctly (Paws::Glacier) and where the appropriate x-amz-glacier-version header is supposedly added (Paws::Net::GlacierCaller) but it doesn't end up in the actual request.

If I hard-code that header into every request then things work properly. I tried but failed to track this through the codebase to figure out what's going wrong.

Is this still an area of activity?

pplu commented 6 years ago

Hi,

I'm not using Glacier API, so haven't haven't dedicated much time to it, but thanks to your report about what's going on, we can get that fixed.

What API calls have you tested to be working when you hardcode the x-amz-glacier-version?

pplu commented 6 years ago

I've just pushed a branch with a fix to the header problem: https://github.com/pplu/aws-sdk-perl/tree/feature/fix-glacier-version. Would you mind trying out your code with this branch? (just shout if you have doubts on how to do this)

It results that the GlacierCaller was just an old draft of what I wanted to do, and the Glacier service was using JsonRestCaller. Sorry for misleading you there.

Also: thanks for the detail of your bug report it helped nail down what had to be fixed. I've also put up a test so that this won't break again.

jvolkening commented 6 years ago

That was quick! I'm away from this at the moment but will test your fix hopefully later today.

jvolkening commented 6 years ago

That commit fixed the above error, but other header(s) are now missing. Currently:

Required parameter missing: partSize

I suspect line 276 in builder-lib/Paws/API/Builder.pm:

 269  sub capitalize_shape {
 270     my ($self, $shape) = @_;
 271 
 272     if ($shape->{ type } eq 'structure'){
 273       foreach my $member (keys %{ $shape->{ members } }){
 274         next if ($member =~ m/^[A-Z]/); # if we already start with capital, don't touch
 275         my $s = delete $shape->{ members }->{ $member };
 276         $s->{ locationName } = $member;
 277         $shape->{ members }->{ $self->capitalize($member) } = $s;
 278       }
 279       $shape->{ required } = [ map { $self->capitalize($_) } @{ $shape->{ required } } ];
 280     }
 281 
 282     return $shape;
 283   };

which seems to be overwriting the locationName with the member name. If I comment out this line and rebuild the module, the correct header is listed in Paws/Glacier/InitiateMultipartUpload and I can use InitiateMultipartUpload() without errors. However, this breaks a whole bunch of stuff in the tests, and the attributes of the returned InitiateMultipartUploadOutput object are undefined.

Nice work on this project, by the way.

pplu commented 6 years ago

I've pushed a version with this hopefully fixed to https://github.com/pplu/aws-sdk-perl/tree/feature/fix-glacier-version. Thanks for the concise information regarding where the bug was happening. It has really determined that this could be fixed quickly.

pplu commented 6 years ago

Hi,

Just a ping to know if everything is working for you

jvolkening commented 6 years ago

Sorry - I thought I had posted an update but apparently that was only in my head.

The headers seem to be working now. However, the return objects are not. I can call

my $response = $ua->InitiateMultipartUpload(...);

but $response->UploadId returns an undefined value.

castaway commented 6 years ago

Tests have been added to ensure that the header mentioned is always sent for Glacier calls. This threw up another issue, documented in #260.

A test for the missing uploadid has been added. The plan to resolve this issue involves comparing RestJsonResponse and RestXMLResponse, which are similar, but not quite the same - RestJsonResponse does not pass the headers to the response object.

Ideally we will extract out the shared parts of the code so that they don't repeat.

castaway commented 6 years ago

I have factored out "response_to_object" in RestXmlResponse and RestJsonResponse, into a ResponseRole class, and then tweaked a couple of things so that the calling mechanism is the same, and the tests continue to pass (had fun with t/25), as does the new Glacier UploadId test..

I feel like a lot more of it could be shared.. https://github.com/shadow-dot-cat/aws-sdk-perl/tree/30_json_response_headers

cventers commented 4 years ago

What would it take to get this work merged into a Paws release? I've started writing code to use Paws for Glacier uploads and am now realizing these issues exist. Is there anything outside contributors can do to assist?

byterock commented 4 years ago

Release 43 in the works adn it is a very large one that should fix many of the problems with S3 and some other actions. This might make it in as well.

Please add in anything you think will help.

I will have to send out some emails to see what the state of the next release is. Things have sort of slowed down the past few weeks

castaway commented 4 years ago

If you could test https://github.com/pplu/aws-sdk-perl/tree/release/0.43 that would be handy, I was in the middle of releasing it and then things went a bit crazy, I'll attempt to get back to it this week.

byterock commented 3 years ago

43 is now (finally) on CPAN

https://metacpan.org/pod/release/BYTEROCK/Paws-0.43/lib/Paws.pm

You might want to try that now.