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

Cached config #1

Closed
ambdisp opened this issue Aug 8, 2019 · 9 comments
Closed

Cached config #1

ambdisp opened this issue Aug 8, 2019 · 9 comments

Comments

@ambdisp
Copy link

ambdisp commented Aug 8, 2019

Thanks, it's very handy package.

I have just noticed, there is an issue with s3 when you cache the config
php artisan config:cache

Exception

GuzzleHttp \ Exception \ ConnectException

Tested on clean Laravel 5.8 installation.

What worked for me:
config.php

...
'aws' => [
    'region' => env('ZIPSTREAM_AWS_REGION', env('AWS_DEFAULT_REGION', 'us-east-1')),
    'key' => env('AWS_ACCESS_KEY_ID', ''),
    'secret' => env('AWS_ACCESS_KEY_KEY', '')
 ]
...

S3File.php

...
/**
 * @return S3Client
 */
public function getS3Client(): S3Client
{
    if (!$this->client) {
        $this->client = new Aws\S3\S3Client([
            'credentials' => [
                'key'    => config('zipstream.aws.key'),
                'secret' => config('zipstream.aws.secret')
            ],
            'region' => $this->getRegion(),
            'version' => '2006-03-01'
        ]);
    }

    return $this->client;
}
...

Hope this helps.

@jszobody
Copy link
Member

jszobody commented Aug 8, 2019

Hmm I wanted the AWS credentials to be handled by the underlying aws-sdk-php package. It looks like I should be creating the S3Client from the pre-wired SDK there.

Can you trying this in S3File real quick:

    public function getS3Client(): S3Client
    {
        if (!$this->client) {
            $this->client = resolve('aws')->createClient('s3', [
                'region' => $this->getRegion(),
                'version' => '2006-03-01'
            ]);
        }
        return $this->client;
    }

That should use the already configured AWS credentials, and merge the specific S3 region for this file (which may or may not match the default region configured).

I'd prefer to use these default configured credentials rather than storing separate ones just for this package, if possible.

@bubba-h57
Copy link
Member

If you relied on https://github.com/aws/aws-sdk-php-laravel then you could simply

$s3 = App::make('aws')->createClient('s3');

or

$s3 = AWS::createClient('s3');

Push handling it even further down the chain of command.

@jszobody
Copy link
Member

jszobody commented Aug 8, 2019

I should really be using S3MultiRegionClient for S3, would simplify this a lot.

@ambdisp
Copy link
Author

ambdisp commented Aug 8, 2019

Thanks for your answers. I appreciate it.
Just a note
All suggestions work once aws/aws-sdk-php-laravel is installed.
Before I had only aws/aws-sdk-php.

@bubba-h57
Copy link
Member

Excellent!

@ianrothmann
Copy link

Hi there!

Still having config issues. it works locally, but not on my Laravel Forge managed server. I've goy aws-sdk-php-laravel installed and configured.

Why not just add the key and secret to the config and use it to instantiate the client if it exists? It would simplify setup a lot.

@jszobody jszobody reopened this Sep 5, 2019
@jszobody
Copy link
Member

jszobody commented Sep 5, 2019

@ianrothmann I just pushed this change, can you try it out? I'm not in a position to test it at the moment. Update your composer.json to use dev-master to get this un-released change, I want to verify it's all working before I release it.

@jszobody
Copy link
Member

@ambdisp and @ianrothmann This is now done. Version 1.5 caches AWS credentials in the package config file, and will work without the aws/aws-sdk-php-laravel specific package. (You still need aws/aws-sdk-php of course).

@ianrothmann
Copy link

Dear @jszobody,

It works like a charm. Thanks for your great work on this package!

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

No branches or pull requests

4 participants