Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add S3 Path to the destination path in saveTo function. #24

Merged
merged 1 commit into from
Aug 7, 2021

Conversation

serich159
Copy link
Contributor

Hello - the $s3Path is ignored in the MediaConvert saveTo function, so that everything is relative to the bucket root. This should fix that up!

@chrisbbreuer
Copy link
Member

Hi @serich159,

Thank you so much for your kind contribution. Nice catch!

We will release a new version with this fix included shortly.

Thanks again!

@chrisbbreuer
Copy link
Member

cc @glennmichael123 let's please take note and see if we can get this covered in our test suite as a regression test 🙏🏼

@chrisbbreuer chrisbbreuer merged commit 6a8365f into meemalabs:main Aug 7, 2021
@glennmichael123
Copy link
Collaborator

@chris1904 Test cases updated and fixed!

@serich159 Thank you so much for your contribution!

However, there was a minor fix I had to add your code cause the path wasn't concatenating properly.

So from:

$destination = 's3://'.($s3bucket ?? config('filesystems.disks.s3.bucket')).$s3Path;

To:

$destination = 's3://'.($s3bucket ?? config('filesystems.disks.s3.bucket')).'/'.$s3Path;

Cheers,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants