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

Per environment lamba configuration JSON files #304

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

winglian
Copy link
Contributor

This PR allows shep to read lambda.{env}.json files in the project and function directories. This should allow more flexibility when deploying to VPCs in different environments, as well as using different runtimes (nodejs4.3, nodejs6.10, etc) in a dev environment before upgrading a prod environment.

Copy link
Contributor

@reconbot reconbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some docs explaining this, other than that, this is great!

@@ -53,11 +53,15 @@ export async function funcs (pattern = '*') {
return funcs
}

export async function lambdaConfig (name) {
export async function lambdaConfig (name, env = null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would you call this without specifying the env? If we never call it that way, might as well throw if either of these arguments are missing

@winglian
Copy link
Contributor Author

@reconbot The only place that might call lambdaConfig without an env parameter would be https://github.com/bustle/shep/blob/master/src/util/load.js#L9. This particular use case is problematic as it is used to get available environments, but requires querying aws for the alias of the deployed function itself. It is probably sufficient to not worry about environment specific configurations for this particular call as long as nobody is throwing any wrenches into the lambda.env.json file such as giving it a new name.

@reconbot
Copy link
Contributor

I think it pays to have two more specific functions instead of an argument. Our code in general is a little overloaded so it's hard to reason when refactoring and that would make it easier. I really appreciate you putting eyes on all that.

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.

2 participants