Skip to content

Revert #12361 and add testing of click flag resoution#12378

Merged
QMalcolm merged 3 commits intomainfrom
qmalcolm--fix-structured-logging-tests-and-begin-testing-click-flags
Jan 22, 2026
Merged

Revert #12361 and add testing of click flag resoution#12378
QMalcolm merged 3 commits intomainfrom
qmalcolm--fix-structured-logging-tests-and-begin-testing-click-flags

Conversation

@QMalcolm
Copy link
Contributor

@QMalcolm QMalcolm commented Jan 21, 2026

Resolves #N/A

Problem

DSI previously had a click upper bound of click<8.3 and core had a lower bound of click>=8.2. This only left two install candidates of click, 8.2.0 and 8.2.1. Unfortunately, those click versions suffer from separate bugs in how flags are resolved 😬 This caused some strife. As such we need to

  1. Guarantee click 8.2.x versions are never installed
  2. Test that click flags don't break like this ever again

Additionally, #12361 added typing to our click flags as an initial attempt to solve the problem. Unfortunately, this caused our CI/CD pipeline to start failing. This is concerning as it implies some edge case behavior change 🙈

Solution

  1. Bump the core click minimum to 8.3.0
  2. Test that click flags work as expected
  3. revert Add typing for bool flag values #12361

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.

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 QMalcolm added the Skip Changelog Skips GHA to check for changelog file label Jan 21, 2026
@QMalcolm QMalcolm requested a review from a team as a code owner January 21, 2026 23:16
@cla-bot cla-bot bot added the cla:yes label Jan 21, 2026
@QMalcolm QMalcolm removed the Skip Changelog Skips GHA to check for changelog file label Jan 21, 2026
@codecov
Copy link

codecov bot commented Jan 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.33%. Comparing base (3bab0f7) to head (37c7e6c).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12378      +/-   ##
==========================================
- Coverage   91.37%   91.33%   -0.05%     
==========================================
  Files         203      203              
  Lines       25092    25092              
==========================================
- Hits        22929    22918      -11     
- Misses       2163     2174      +11     
Flag Coverage Δ
integration 88.14% <ø> (-0.12%) ⬇️
unit 65.31% <ø> (ø)

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

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

@QMalcolm QMalcolm merged commit 9c1bb27 into main Jan 22, 2026
55 of 56 checks passed
@QMalcolm QMalcolm deleted the qmalcolm--fix-structured-logging-tests-and-begin-testing-click-flags branch January 22, 2026 14:27
QMalcolm added a commit that referenced this pull request Feb 2, 2026
* 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
…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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants