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

Module broken #29

Closed
ocervell opened this issue Dec 20, 2019 · 6 comments · Fixed by #30
Closed

Module broken #29

ocervell opened this issue Dec 20, 2019 · 6 comments · Fixed by #30

Comments

@ocervell
Copy link
Contributor

Last change from #26 (changing variable scheduler_job from any to object({name=string}) has broken the module.

I'm getting the error:

Error: Inconsistent conditional result types

  on .terraform/modules/slo-app-search-latency-64ms.slo_cloud_function/terraform-google-modules-terraform-google-scheduled-function-7aacc62/outputs.tf line 23, in output "scheduler_job":
  23:   value       = var.scheduler_job == null ? google_cloud_scheduler_job.job : var.scheduler_job
    |----------------
    | google_cloud_scheduler_job.job is tuple with 1 element
    | var.scheduler_job is null

The true and false result expressions must have consistent types. The given
expressions are tuple and object, respectively.
@aaron-lane
Copy link
Contributor

I don't think changing the type of var.scheduler_job is relevant to that error. The issue appears to be that we're outputting google_cloud_scheduler_job.job which is tuple because that resource has a count. We should be outputting google_cloud_scheduler_job.job[0].

@ocervell
Copy link
Contributor Author

ocervell commented Dec 20, 2019

I am doing some testing, but it seems I'm getting the same error with google_cloud_scheduler_job.job.0 (the outcome is: "google_cloud_scheduler_job.job.0 is an object with 12 elements" error)

@aaron-lane
Copy link
Contributor

We should reconsider outputting a variable as it does not impact the dependency graph in a meaningful way and removing it from the output value would avoid this problem.

@ocervell
Copy link
Contributor Author

The goal of the PR was to use this output variable to allow sharing a Cloud Scheduler with other modules, so this is needed.

ocervell pushed a commit to ocervell/terraform-google-scheduled-function that referenced this issue Dec 20, 2019
@ocervell ocervell mentioned this issue Dec 20, 2019
@aaron-lane
Copy link
Contributor

I understand the intent of the new feature; it's just that there is no value in conditionally outputting the input variable. All modules which need to use the same job should use the job output from the module which creates it. Passing that output in and out of each chained module does not enforce a meaningful dependency, and creates problems as we see with the type.

@ocervell
Copy link
Contributor Author

ocervell commented Dec 20, 2019

Ok, let's default to null if no scheduler is created for the output and enforce a strict type checking.

morgante added a commit that referenced this issue 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 a pull request may close this issue.

2 participants