Skip to content

Add typing for bool flag values#12361

Merged
QMalcolm merged 2 commits intomainfrom
will/add-type-flag-options
Jan 19, 2026
Merged

Add typing for bool flag values#12361
QMalcolm merged 2 commits intomainfrom
will/add-type-flag-options

Conversation

@WilliamDee
Copy link
Contributor

@WilliamDee WilliamDee commented Jan 18, 2026

Seems like with the version bump of click, flags are now rendering incorrectly without the type=click.BOOL being set. Ie.,

export FLAG_ENV_VAR=FALSE -> resolves to True

and none of the click falsey values are being respected (ie., 0, false, off).

Problem

Solution

Checklist

  • I have read the contributing guide and understand what's expected of me.
  • I have run this code in development, and it appears to resolve the stated issue.
  • This PR includes tests, or tests are not required or relevant for this PR.
  • This PR has no interface changes (e.g., macros, CLI, logs, JSON artifacts, config files, adapter interface, etc.) or this PR has already received feedback and approval from Product or DX.
  • This PR includes type annotations for new and modified functions.

@WilliamDee WilliamDee requested a review from a team as a code owner January 18, 2026 23:28
@cla-bot cla-bot bot added the cla:yes label Jan 18, 2026
@github-actions github-actions bot added the community This PR is from a community member label Jan 18, 2026
@codecov
Copy link

codecov bot commented Jan 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.36%. Comparing base (2a96f8c) to head (1290609).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #12361   +/-   ##
=======================================
  Coverage   91.36%   91.36%           
=======================================
  Files         203      203           
  Lines       25092    25092           
=======================================
  Hits        22925    22925           
  Misses       2167     2167           
Flag Coverage Δ
integration 88.23% <ø> (-0.01%) ⬇️
unit 65.30% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
Unit Tests 65.30% <ø> (ø)
Integration Tests 88.23% <ø> (-0.01%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@QMalcolm QMalcolm left a comment

Choose a reason for hiding this comment

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

@QMalcolm QMalcolm merged commit 7634345 into main Jan 19, 2026
60 checks passed
@QMalcolm QMalcolm deleted the will/add-type-flag-options branch January 19, 2026 00:14
QMalcolm added a commit that referenced this pull request Jan 21, 2026
QMalcolm added a commit that referenced this pull request Jan 21, 2026
This reverts commit 7634345. We're
doing this because that fix wasn't actually what fixed our problem,
and it seems to be causing some edge case with our structured logging
testing environment. That itself worries me that this has created
some edge case behavior change that we are unaware of.
QMalcolm added a commit that referenced this pull request Jan 22, 2026
* Revert "Add typing for bool flag values (#12361)"

This reverts commit 7634345. We're
doing this because that fix wasn't actually what fixed our problem,
and it seems to be causing some edge case with our structured logging
testing environment. That itself worries me that this has created
some edge case behavior change that we are unaware of.

* Add testing of click flag resolution

* Bump minimum `click` version to 8.3.0
abhishek09827 pushed a commit to abhishek09827/dbt-core that referenced this pull request Feb 19, 2026
* Add type=click.BOOL for all flag options

* changelog
abhishek09827 pushed a commit to abhishek09827/dbt-core that referenced this pull request Feb 19, 2026
…labs#12378)

* Revert "Add typing for bool flag values (dbt-labs#12361)"

This reverts commit 7634345. We're
doing this because that fix wasn't actually what fixed our problem,
and it seems to be causing some edge case with our structured logging
testing environment. That itself worries me that this has created
some edge case behavior change that we are unaware of.

* Add testing of click flag resolution

* Bump minimum `click` version to 8.3.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla:yes community This PR is from a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants