Skip to content
This repository has been archived by the owner on Nov 24, 2021. It is now read-only.

Support environment variables? #1

Closed
JedWatson opened this issue Aug 3, 2016 · 1 comment
Closed

Support environment variables? #1

JedWatson opened this issue Aug 3, 2016 · 1 comment

Comments

@JedWatson
Copy link
Member

Keystone currently supports detecting S3 options in process.env - see https://github.com/keystonejs/keystone/blob/master/index.js#L75-L77

I think it would be worth continuing to support these, like this: https://github.com/keystonejs/keystone-email/blob/master/lib/transports/mailgun/getSendOptions.js#L7-L12

We may remove support in Keystone for the s3 config option though..? Would probably be awkward to support and breaks separation of concerns between the packages

@josephg
Copy link
Contributor

josephg commented Aug 3, 2016

Yeah I was wondering about that while writing it. The test suite uses environment variables.

I think generally it should be up to the application to configure the S3 options. The question is between writing:

var storage = new keystone.Storage({
    adapter: require('keystone-s3'),
    s3: {
        key: process.env.S3_KEY,
        secret: process.env.S3_SECRET,
        bucket: process.env.S3_BUCKET,
    },
})

... or leaving that out. Tbh I can't think of any strong reason to leave it out - if you want to manually configure those properties you can either not specify environment variables or just override them in your configuration.

@josephg josephg closed this as completed in 2474635 Aug 3, 2016
dominikwilkowski pushed a commit that referenced this issue Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants