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

[CT-559] [Bug] Partial parser does not recognize default values in envvars #5155

Closed
1 task done
ajbosco opened this issue Apr 26, 2022 · 4 comments · Fixed by #5589
Closed
1 task done

[CT-559] [Bug] Partial parser does not recognize default values in envvars #5155

ajbosco opened this issue Apr 26, 2022 · 4 comments · Fixed by #5589
Assignees
Labels
bug Something isn't working partial_parsing

Comments

@ajbosco
Copy link

ajbosco commented Apr 26, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

The partial parser does not support default values for envvars. As a result, it will consider all sources deleted since the current value is None even if the default value and previous value are the same and unchanged.

if current_value is None:

Expected Behavior

Since nothing has been changed in the files, I would expect the partial parser to detect the default value matches the previous value and not reparse all the files.

Steps To Reproduce

Include an envvar like this in a source YAML file:

schema: '{{ env_var("OUR_SCHEMA_ENV_VAR", "default_value") }}'

Relevant log output

No response

Environment

- OS: Ubuntu 20.04
- Python: 3.9.12
- dbt: 1.0.4

What database are you using dbt with?

No response

Additional Context

No response

@ajbosco ajbosco added bug Something isn't working triage labels Apr 26, 2022
@github-actions github-actions bot changed the title [Bug] Partial parser does not recognize default values in envvars [CT-559] [Bug] Partial parser does not recognize default values in envvars Apr 26, 2022
@jtcohen6
Copy link
Contributor

@ajbosco Thanks for opening! Original thread with dbt parse output: #3164 (comment). It sounds like this might save your team up to 2 min on each re-parse.

I'll let someone from the Language team offer their sense of what a fix here might require.

@gshank
Copy link
Contributor

gshank commented Apr 28, 2022

The code that stores the env_var value will actually use the default value if one is provided. So I'm not exactly sure why this wouldn't be working. I'll have to do some investigation.

@gshank gshank removed the triage label Apr 28, 2022
@gshank
Copy link
Contributor

gshank commented Apr 28, 2022

Oh, I see. We do save the default, but os.getenv won't find it, so it looks different.

I think in order to change that we'd have to save the fact that it uses a default. It would be difficult to handle multiple calls to the same env_var with different defaults.

@gshank
Copy link
Contributor

gshank commented Apr 28, 2022

Maybe instead of saving the default value in the env_var, we could save something like "DEFAULT" and then not compare those env_vars, since presumably if the default changed, the file itself would be marked as changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working partial_parsing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants