robbiepaul / cloudconvert-laravel

A Laravel wrapper for the CloudConvert API
177 stars 34 forks source link

Queue and saving to S3 #52

Closed ghost closed 7 years ago

ghost commented 7 years ago

Hi, cheers for making this!

I'm having a problem getting queues to work with Amazon S3. When I do this:

CloudConvert::file(asset(Storage::url($path)))->queue('to', CloudConvert::S3($thumb_full_name));

I get back this error: {"error":"This conversiontype is not supported! (pdf to AKIAJTZQLLWBR2GD72OQ)","code":400}

If I change the file name from the variable to test.jpg I still get that error. Should I be using the queue method in a different way to get it to work?

Cheers!

ghost commented 7 years ago

Without using queue I get this error when using S3. Must be related I guess.

Saving to S3 failed: Unexpected key 'outputformat' found in params (Code: UnexpectedParameter)

robbiepaul commented 7 years ago

The queue method shouldn't really be combined with saving to S3 as saving to S3 is a non-blocking process anyway.

Did you try:

CloudConvert::file(asset(Storage::url($path)))->to(CloudConvert::S3($thumb_full_name));

Can you confirm what the values of asset(Storage::url($path)) and $thumb_full_name are?

ghost commented 7 years ago

@robbiepaul I changed that to use $request->file now instead but am still getting that outputformat error.

ghost commented 7 years ago

Sorry so now I'm using:

CloudConvert::file(Storage::url($path))->to(CloudConvert::S3('test.jpg'));

Maybe it's because I'm using S3 for my storage already?

robbiepaul commented 7 years ago

It should work if the URL is publicly viewable. The CloudConvert API will simply download the asset from your S3 bucket and convert it.

Not sure why it's saying outputformat is an unexpected key, it's on the docs and it's been working previously. I'll run some tests tonight

ghost commented 7 years ago

@robbiepaul Thanks! Very confused too. I'm able to upload to S3 normally through laravel so I think the credentials are okay and cloudconvert works grand when I store the files on the local disk.

The only thing I can think it might be is the ACL value, I've left it blank but it's an optional parameter.

ghost commented 7 years ago

@robbiepaul Did you manage to get a chance to look at this? I sent cloudconvert the error report last week but not sure if they'll get back to me.

Cheers!

josiasmontag commented 7 years ago

I had a look into your error report @LindenWalsh. The problem is that the laravel wrapper is sending the following request to CloudConvert:

{  
   "file":"...",
   "filename":"...pdf",
   "input":"download",
   "output":{  
      "s3":{  
         "accesskeyid":"...",
         "acl":"public-read",
         "bucket":"...",
         "outputformat":null,
         "path":"....jpg",
         "region":"eu-west-1",
         "secretaccesskey":"..."
      }
   },
   "outputformat":"jpg",
   "preset":null,
   "wait":false
}

The problem is output.s3.outputformat. This key is not allowed here and results in the mentioned error message.

ghost commented 7 years ago

@josiasmontag Thank you!

@robbiepaul I can't see where that's being set in the wrapper.

robbiepaul commented 7 years ago

Thanks @josiasmontag

It seems to be an issue with get_object_vars returning protected variables (on this line https://github.com/robbiepaul/cloudconvert-laravel/blob/2.x/src/RobbieP/CloudConvertLaravel/Storage.php#L23).

I can't reproduce the issue locally, however I'm trying it outside Laravel. I'll have another try but with a Laravel project

@LindenWalsh What PHP version/OS are you on?

ghost commented 7 years ago

@robbiepaul Cheers! I'm running PHP Version 7.1.0. I'm using Valet and I'm running it locally on Yosemite

robbiepaul commented 7 years ago

@LindenWalsh perfect thanks, I'll investigate

ghost commented 7 years ago

@robbiepaul Nice one, thank you!

robbiepaul commented 7 years ago

@LindenWalsh Can I just check how you're starting the conversion?

I just successfully ran:

CloudConvert::file('http://www.pdf995.com/samples/pdf.pdf')->to(CloudConvert::S3('test.jpg'));

On PHP 7.1 using Valet

Can you post your code?

ghost commented 7 years ago

@robbiepaul

CloudConvert::file(Storage::url($path))->to(CloudConvert::S3($thumb_full_name));
robbiepaul commented 7 years ago

@LindenWalsh and what's the value of $thumb_full_name ?

ghost commented 7 years ago

@robbiepaul It was filename--thumb.jpg

I ran your example too and got the same error as before. Looks like there's something weird going on on my end then

robbiepaul commented 7 years ago

it's strange @LindenWalsh. What version of Laravel you on? I can try that

I ran it on a clean Laravel 5.3 install

ghost commented 7 years ago

@robbiepaul I'm using 5.3 as well.

robbiepaul commented 7 years ago

@LindenWalsh strange that I can't reproduce it. I'll submit a patch for what I think it might be, however you'll have to let me know if it works

ghost commented 7 years ago

@robbiepaul Awesome, thanks and will do!

robbiepaul commented 7 years ago

@LindenWalsh I've pushed up a patch in this commit 32af0aa3ee036f47ae6d51838b6685cff2e7d5f3 tagged as v2.3.3

Could you run a composer update robbiep/cloudconvert-laravel?

ghost commented 7 years ago

@robbiepaul You're a legend, that worked!

FINISHED Saved file to S3

Thank you!

robbiepaul commented 7 years ago

@LindenWalsh No worries, I'm still a little confused why I couldn't recreate the error but hey, it works!

ghost commented 7 years ago

@robbiepaul It's a weird one! Thanks again!