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

core: Report reason for deferring data read until apply #30971

Merged
merged 1 commit into from May 9, 2022

Conversation

apparentlymart
Copy link
Member

@apparentlymart apparentlymart commented Apr 30, 2022

We have two different reasons why a data resource might be read only during apply, rather than during planning as usual: the configuration contains unknown values, or the data resource as a whole depends on a managed resource which itself has a change pending.

However, we didn't previously distinguish these two in a way that allowed the UI to describe the difference, and so we confusingly reported both as "config refers to values not yet known", which in turn led to a number of reasonable questions about why Terraform was claiming that but then immediately below showing the configuration entirely known.

Now we'll use our existing ActionReason mechanism to tell the UI layer which of the two reasons applies to a particular data resource instance. The "dependency pending" situation tends to happen in conjunction with "config unknown", so we'll prefer to refer that the configuration is unknown if both are true.


The plan hint for the config-unknown case remains unchanged:

  # data.test_instance.example will be read during apply
  # (config refers to values not yet known)
 <= data "test_instance" "example" {
        name = "name"
    }

For the "dependencies pending" case there is now a new parenthetical hint:

  # data.test_instance.example will be read during apply
  # (depends on a resource or a module with changes pending)
 <= data "test_instance" "example" {
        name = "name"
    }

I had originally hoped to include one or more of the specific resources that the data resource depends on as part of this output, but that isn't possible at the moment because we don't currently preserve dependency information directly inside the plan; instead, during the apply phase we reconstruct the dependency graph from the source material.

We have two different reasons why a data resource might be read only
during apply, rather than during planning as usual: the configuration
contains unknown values, or the data resource as a whole depends on a
managed resource which itself has a change pending.

However, we didn't previously distinguish these two in a way that allowed
the UI to describe the difference, and so we confusingly reported both
as "config refers to values not yet known", which in turn led to a number
of reasonable questions about why Terraform was claiming that but then
immediately below showing the configuration entirely known.

Now we'll use our existing "ActionReason" mechanism to tell the UI layer
which of the two reasons applies to a particular data resource instance.
The "dependency pending" situation tends to happen in conjunction with
"config unknown", so we'll prefer to refer that the configuration is
unknown if both are true.
@jbardin
Copy link
Member

jbardin commented May 2, 2022

Since this is essentially a minor UI change, and may merge poorly due to proto/stringer changes, perhaps we should backport this to v1.2 before the rc?

@apparentlymart
Copy link
Member Author

I'm going to hold on backporting this into v1.2 for the moment because rc1 already happened and so we're currently in a freeze period. However, we can consider backporting this into the v1.2 branch manually later if it proves useful to do so; I don't have any particular objection to doing so.

@apparentlymart apparentlymart merged commit 4cffff2 into main May 9, 2022
@apparentlymart apparentlymart deleted the f-data-defer-feedback branch May 9, 2022 18:12
@github-actions
Copy link

github-actions bot commented May 9, 2022

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

@github-actions
Copy link

github-actions bot commented Jun 9, 2022

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants