Skip to content

infra: propagate custom timeouts to Cloud Workflows preprocessing and postprocessing steps #172

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

Open
dwnoble wants to merge 1 commit into
base: main
Choose a base branch
from
Open

infra: propagate custom timeouts to Cloud Workflows preprocessing and postprocessing steps #172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions infra/dcp/modules/ingestion/workflow/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ resource "google_workflows_workflow" "ingestion_orchestrator" {
dataflow_ip_configuration = var.dataflow_ip_configuration
dataflow_subnetwork = var.dataflow_subnetwork
embeddings_timeout = var.embeddings_timeout
preprocessing_job_timeout = var.preprocessing_job_timeout
postprocessing_job_timeout = var.postprocessing_job_timeout
clean_namespace_prefix = local.clean_namespace_prefix


enable_redis_cache_clearing = var.enable_redis_cache_clearing
preprocessing_job_name = var.preprocessing_job_name
postprocessing_job_name = var.postprocessing_job_name
Expand Down
14 changes: 14 additions & 0 deletions infra/dcp/modules/ingestion/workflow/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,20 @@ variable "embeddings_timeout" {
default = 1800
}

variable "preprocessing_job_timeout" {
type = number
description = "Timeout in seconds for the preprocessing Cloud Run job execution"
default = 21600
}

variable "postprocessing_job_timeout" {
type = number
description = "Timeout in seconds for the postprocessing Cloud Run job execution"
default = 21600
}



variable "ingestion_helper_service_name" {
type = string
description = "Name of the ingestion helper Cloud Run service"
Expand Down
7 changes: 7 additions & 0 deletions infra/dcp/modules/ingestion/workflow/workflow.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,10 @@ run_postprocessing_job:
- "--import_list"
- '$${json.encode_to_string(import_list)}'
- "--no-dry_run"
connector_params:

@gmechali gmechali Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A small nit, I dont think you're propagating these to the actual pre/post processing jobs as the timeout, it's just the polling connection.

You also can pass in these timeouts in the cloud run job containers running these.

Maybe passing it in both the cloud run job + the polling connector is the right approach? Can you update the worfklow.yaml connector params on the preprocessing polling too? I hardcoded that value (3993c71)

timeout: ${postprocessing_job_timeout}
result: run_res

- call_poll_postprocessing:
call: poll_cloud_run_job_execution
args:
Expand Down Expand Up @@ -520,8 +523,12 @@ launch_and_poll_preprocessing_job:
- name: 'DATA_RUN_MODE'
value: 'dcpbridge'
args: '$${container_args}'
connector_params:
timeout: ${preprocessing_job_timeout}
result: run_res


@gmechali gmechali Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: remove extra lines


- assign_execution_name:
assign:
- execution_name: '$${run_res.metadata.name}'
Expand Down
3 changes: 3 additions & 0 deletions infra/dcp/modules/stack/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,13 @@ module "ingestion_workflow" {
dataflow_template_gcs_path = var.ingestion_config.dataflow_template_gcs_path
preprocessing_job_name = var.ingestion_config.enable_ingestion ? module.ingestion_preprocessing_job[0].job_name : ""
postprocessing_job_name = var.ingestion_config.enable_ingestion ? module.ingestion_postprocessing_job[0].job_name : ""
preprocessing_job_timeout = tonumber(replace(var.ingestion_config.preprocessing_job_timeout, "s", ""))
postprocessing_job_timeout = tonumber(replace(var.ingestion_config.postprocessing_job_timeout, "s", ""))
Comment on lines +227 to +228

@gemini-code-assist gemini-code-assist Bot Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If preprocessing_job_timeout or postprocessing_job_timeout is null or empty (e.g., when ingestion is disabled or timeouts are omitted), replace() or tonumber() will throw a Terraform evaluation error.

Using try() provides a safe fallback to null, allowing the downstream workflow module to use its own default values.

  preprocessing_job_timeout      = try(tonumber(replace(var.ingestion_config.preprocessing_job_timeout, "s", "")), null)
  postprocessing_job_timeout     = try(tonumber(replace(var.ingestion_config.postprocessing_job_timeout, "s", "")), null)
References
  1. When designing nested Terraform modules, set the top-level variable default to null and declare the attribute as optional in intermediate module configuration objects. This ensures that the default value defined in the innermost module is respected and not overridden by parent defaults.


depends_on = [module.ingestion_helper_service]
}


module "redis" {
source = "../redis"
count = var.redis_config.enable ? 1 : 0
Expand Down
Loading