Skip to content

Update main.tf #4

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Update main.tf #4

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7515ae2
Update main.tf
goruha Jan 29, 2019
f775f72
Update main.tf
goruha Jan 29, 2019
dbcdf33
Update main.tf
goruha Jan 29, 2019
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Update main.tf
  • Loading branch information
@goruha
goruha authored Jan 29, 2019
commit dbcdf33c494d960724d293d3212a7c5fee890ac0
2 changes: 1 addition & 1 deletion main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ module "artifact" {
filename = "lambda.zip"
module_name = "terraform-aws-lambda-elasticsearch-cleanup"
module_path = "${path.module}"
git_ref = "0.1.2"
git_ref = "e7633d0eaef2200bbe0d3dfdd39a87a904c88084"
Copy link
Contributor

@joshmyers joshmyers Jan 29, 2019

Choose a reason for hiding this comment

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

We shouldn't hardcode this here, it will change. We could optionally pass it from outside the module, if we want to override the current head git SHA? We build and deploy this zip file as an S3 artifact on every commit via Travis and the terraform-external-module-artifact module checks the HEAD commit of the module and looks for an artifact there. This all came about when conversing about how to build/deploy Lambda artifacts. We had three different proposals:

  • Build and upload as a Lambda zip at Terraform run time
  • Check a prebuild zip into git
  • Have some kind of artifact build and deploy zip file out of band of Terraform and without the need to check binaries into git.

@osterman wanted to go with the last option and made https://github.com/cloudposse/terraform-external-module-artifact to do so. Do you know why you were getting the 404 when trying to run this module? e.g. what was the curl command?
Copy link
Contributor

@joshmyers joshmyers Jan 29, 2019

Choose a reason for hiding this comment

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

Part of the problem of the last approach is that it was very difficult / nay impossible to do local dev as the thing hadn't been build and pushed yet.
Copy link
Member Author

@goruha goruha Jan 29, 2019

Choose a reason for hiding this comment

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

@joshmyers

  1. I do not want to lookup correct SHA when going to install that.
  2. SHA is kind of version, we pin version of modules, why we can not pin it as SHA?
  3. I'm ok to define variable elasticsearch_cleanup_version with default value point to the SHA
Copy link
Member Author

@goruha goruha Jan 29, 2019

Choose a reason for hiding this comment

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

I do not know what curl command was
Copy link
Contributor

@joshmyers joshmyers Jan 29, 2019

Choose a reason for hiding this comment

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

💯 you shouldn't need to lookup a commit sha when wanting to deploy this lambda function
Copy link
Member

@osterman osterman Jan 29, 2019

Choose a reason for hiding this comment

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

this works without needing to hardcode. We had this working. Something else might have changed, but the strategy works.
Copy link
Member

@osterman osterman Jan 29, 2019

Choose a reason for hiding this comment

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

Part of the problem of the last approach is that it was very difficult / nay impossible to do local dev as the thing hadn't been build and pushed yet.

It is local dev because we allow all paths to be defined as inputs to the module. So the guy who knows what they are doing can... But it's also possible for someone without nodejs installed locall to deploy this module because it defaults to remote artifacts. =)
}

# Locals
Expand Down