Skip to content

Fix!: mark vars referenced in metadata macros as metadata #4936

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

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

Conversation

georgesittas
Copy link
Contributor

Macro variable references are always treated as non-metadata today. This means that if, for example, a variable is referenced within a metadata-only macro, changing its value will result in a breaking change, which is inconsistent.

This PR alters this behavior, similar to the macro metadata-only status propagation:

  • Variables referenced within metadata-only macro definitions can be treated as metadata-obly
  • Variables referenced in metadata-only macro calls can be treated as metadata-only
  • Variables referenced within metadata properties can be treated as metadata-only

I intentionally say "can" instead of "will" above, because we need to factor in all references of a variable to decide whether it's a metadata-only reference. The rules implemented here are similar to those we apply for macros: a non-metadata occurrence overrules all metadata occurrences.

Additionally, this PR introduces trimming for blueprint variables. Certain blueprint variables, e.g. used in model names, aren't required after loading, while others are because they may be referenced in the model's statements or in "runtime-rendered" properties (e.g., merge_filter).

The former category can be omitted from the model's python_env, thus reducing its snapshot's size, as long as a variable is only referenced in the meta block and in fields that are static after loading the model.

Both of these changes are quite breaking, so I'm planning to implement a migration script to at least warn about this. I'm also planning to increase the testing coverage.

@georgesittas georgesittas marked this pull request as draft July 8, 2025 15:32
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