-
Notifications
You must be signed in to change notification settings - Fork 1
Fix: better handling of non-iterable dbt result objects and selectable dbt commands #143
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
Conversation
v0.3.33 release
v0.3.34 release
v0.3.35 release
v0.3.36 release
v0.3.37 release
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes crashes in dbt command handling by addressing two issues: commands incorrectly receiving the --select flag when they don't support it (e.g., dbt deps), and the app attempting to iterate over non-iterable result objects from commands like dbt docs generate.
- Added
COMMANDS_WITH_SELECTfrozenset to explicitly control which commands receive the--selectflag - Enhanced
_log_run_resultsto handle both iterable (RunExecutionResult) and non-iterable result types (CatalogArtifact, Manifest, bool, None) - Added type checking and debug logging for non-iterable results to prevent crashes
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM other than the missing test coverage as suggested.
Thanks for the patch!
Summary
This PR improves dbt command handling in Tower CLI with better handling of commands that accept a
--selectargument, and also handles the different result types of those commands (iterable vs non-iterable), to prevent the app crashing.Current issues
1. Commands that don't support
--selecthave it added to themImagine I want to build a model called
stg_economic_booked_invoices. In a dbt job,dbt depswill be run to install package dependencies, before proceeding withdbt buildApp run result
What is happening above is that when I chose the
DBT_SELECT, it passes this to dbt commands that don't support this, likedbt deps, causing a crash.2. Non-iterable result objects attempting to be iterated over
When I add
docs generateas one of theDBT_COMMANDSenv vars, it will fail the app run due to the result ofdbt docs generatenot being an iterable object (unlike the result ofdbt build). I get:TypeError: 'CatalogArtifact' object is not iterableApp run result
Changes
1. Selector flag improvements:
COMMANDS_WITH_SELECTfrozenset to explicitly define which dbt commands support the--selectflag (run, test, build, compile, seed, snapshot, docs, list/ls, show, source) – these can be found here: https://docs.getdbt.com/reference/node-selection/syntaxrun_dbt_workflowto only add--selectflag to commands that support it, preventing errors with commands like docs generate, clean, deps, etc.2. Result logging improvements:
_log_run_resultsfunction to handle different dbt command return types correctlyRunExecutionResult(run, test, build, etc.)CatalogArtifact,Manifest,bool,None)3. Comprehensive test coverage:
COMMANDS_WITH_SELECTto ensure they properly receive the--selectflag when a selector is provided--select(deps, clean, debug, init) to verify the flag is not added--selectflags when already present in command argsTestRunDbtWorkflowclassImpact
These changes fix potential crashes when running certain dbt commands – particularly
dbt deps– that don't support--select, as well as crashes from those commands that return non-iterable results.