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

fix(variable): change 'value' type to Dynamic #277

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mitchnielsen
Copy link
Contributor

@mitchnielsen mitchnielsen commented Oct 4, 2024

Summary

Changes the Variable 'value' type from string to Dynamic.

In PrefectHQ/prefect#13543 and associated changes around May 2024, Variables were updated from simple strings to JSON objects. The Terraform provider has still been treating them as strings, so when folks tried to put JSON-compatible values in them, Terraform would fail to work with them as found in #254

That said, this doesn't mean you can always expect a full JSON object in response. It could be individual JSON-compatible types, like bool or int. So we actually need a Dynamic attribute type here.

Related to https://linear.app/prefect/issue/PLA-247/changing-variable-to-a-json-value-in-the-ui-makes-next-terraform-run

Related to #254

References

From https://docs.prefect.io/3.0/develop/variables:

(Variables) are named, mutable values of any JSON type

Also, you can see the accepted types for the value field here:

$ curl -s https://api.prefect.cloud/api/openapi.json | jq '.components.schemas.Variable.properties.value.anyOf'
$ curl -s https://api.prefect.cloud/api/openapi.json | jq '.components.schemas.Variable.properties.value.anyOf[]'
{
  "type": "string",
  "maxLength": 5000
}
{
  "type": "integer"
}
{
  "type": "number"
}
{
  "type": "boolean"
}
{
  "type": "object"
}
{
  "items": {},
  "type": "array"
}

Changes the Variable 'value' type from string to JSON.

In PrefectHQ/prefect#13543 and associated
changes around May 2024, Variables were updated from simple strings to
JSON objects. The Terraform provider has still been treating them as
strings, so when folks tried to put JSON-compatible values in them,
Terraform would fail to work with them as found in
#254

Related to https://linear.app/prefect/issue/PLA-247/changing-variable-to-a-json-value-in-the-ui-makes-next-terraform-run

Related to #254
@mitchnielsen
Copy link
Contributor Author

So changing this to a JSON type may not be the full fix here. As the API response shows, the value here can be almost anything: string, integer, number, boolean, object, and array. So if I put 5 in the Variable, the provider will fail to convert that into map[string]interface. Need to think about this some more.

@mitchnielsen
Copy link
Contributor Author

We may need to use a dynamic attribute here: https://developer.hashicorp.com/terraform/plugin/framework/handling-data/attributes/dynamic

@mitchnielsen mitchnielsen changed the title fix(variable): change 'value' type to JSON fix(variable): change 'value' type to Dynamic Oct 4, 2024
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.

1 participant