Skip to content

Add ingestion_input_path to top level tf outputs#72

Open
clincoln8 wants to merge 3 commits into
datacommonsorg:mainfrom
clincoln8:expose-input-path
Open

Add ingestion_input_path to top level tf outputs#72
clincoln8 wants to merge 3 commits into
datacommonsorg:mainfrom
clincoln8:expose-input-path

Conversation

@clincoln8
Copy link
Copy Markdown
Contributor

No description provided.

@clincoln8 clincoln8 requested a review from gmechali May 22, 2026 23:55
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds a new Terraform output ingestion_input_path to the infra/dcp/outputs.tf file. The reviewer suggests making this output conditional on the enable_ingestion variable to ensure consistency and prevent misleading information when the feature is disabled, along with a recommendation to update the description and document stack dependencies.

Comment thread infra/dcp/outputs.tf
Comment on lines +63 to +66
output "ingestion_input_path" {
description = "Path within the bucket where raw files should be uploaded"
value = var.ingestion_input_path
}
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.

medium

To maintain consistency with other outputs in this project and avoid providing misleading information when the ingestion feature is disabled, this output should be conditional on var.enable_ingestion. Additionally, the description should be updated to match the one used in the variable definition for consistency. Since this output relies on the ingestion_bucket_url from another stack, ensure this dependency is explicitly documented in the Terraform variable descriptions or comments to prevent confusion regarding its optionality, as per repository rules.

output "ingestion_input_path" {
  description = "Path within the bucket where raw files are uploaded"
  value       = var.enable_ingestion ? var.ingestion_input_path : null
}
References
  1. When an infrastructure component or workflow step has a mandatory dependency on another stack, ensure this requirement is explicitly documented in Terraform variable descriptions or comments to prevent confusion regarding its optionality.

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