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

Feature/7 re use new scheduled function #9

Conversation

taylorludwig
Copy link
Contributor

@taylorludwig taylorludwig commented Dec 14, 2019

Depends on
terraform-google-modules/terraform-google-event-function#38
and
terraform-google-modules/terraform-google-scheduled-function#28

Being merged and released first. Then we can update module source codes to move off of my forks.

  • Updates the slo module to use the scheduled-function module now that it has proper support for waiting for local_file creations before creating the archive for the cloud function

  • Updates slo-pipeline to use event-function module instead of duplicated functionality. Also relies on new local_file to archive dependency functionality

  • Updates the setup and tests to pass

@taylorludwig
Copy link
Contributor Author

We could possibly add some better tests to verify the functions work as intended.

But after applying locally I have verified both the scheduled and event driven functions are being triggered. And I do see data showing up in bigquery for the slo-pipeline

@taylorludwig taylorludwig self-assigned this Dec 17, 2019
Copy link

@ocervell ocervell left a comment

Choose a reason for hiding this comment

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

LGTM, need one change once terraform-google-scheduled-function PR is merged. For the tests, we can add them in a further PR. Good work !

modules/slo/main.tf Outdated Show resolved Hide resolved
test/setup/versions.tf Show resolved Hide resolved
test/setup/versions.tf Show resolved Hide resolved
@ocervell ocervell marked this pull request as ready for review December 18, 2019 11:24
@ocervell
Copy link

LGTM

@ocervell ocervell merged commit b9862d0 into terraform-google-modules:master Dec 20, 2019
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