[dagster-dbt] Support dbt selections for dbt Cloud

[maximearmstrong]

Summary & Motivation

This PR updates the code to allow users to pass select and exclude strings to the dbt_cloud_assets decorator, and the spec loader functions for dbt Cloud.

How I Tested These Changes

Additional tests with BK

[maximearmstrong]

• [dagster-dbt] Support asset checks in dbt_cloud_assets #28907
• [dagster-dbt] Support dbt selections for dbt Cloud #28906 👈 (View in Graphite)
• [dagster-dbt] Support context in default asset events for dbt Cloud #28905
• [dagster-dbt] Update args with context in DbtCloudWorkspace.cli(..) #28902
• [dagster-dbt] Update DbtCloudCliInvocation to accept manifest #28901
• [dagster-dbt] Implement basic dbt_cloud_assets decorator #28886
• [dagster-dbt] Implement DbtCloudCliInvocation #28152
• [5/n][dagster-dbt] Implement load_dbt_cloud_asset_specs and load_dbt_cloud_check_specs #28151
• [4/n][dagster-dbt] Implement fetch_workspace_data for DbtCloudWorkspace #28150
• [3/n][dagster-dbt] Add artifact methods in DbtCloudJobRunHandler #28149
• [2/n][dagster-dbt] Add DbtCloudJobRunHandler #28148
• [1/n][dagster-dbt] Add DbtCloudWorkspace and DbtCloudWorkspaceClient for dbt Cloud v2 #28147
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[graphite-app]

There's an inconsistency in the handling of the exclude parameter. While load_specs declares it as Optional[str] = None, when passing to DbtCloudWorkspaceDefsLoader, it uses exclude or "".

This approach has a potential issue: if exclude is a valid selection string that evaluates to False in Python (like "0" or "False"), it would incorrectly be replaced with an empty string.

Consider replacing:

exclude or ""

with:

exclude if exclude is not None else ""

This ensures that only None values are converted to empty strings, while preserving all other valid selection strings regardless of their truthiness.

Suggested change

	select=select,
	exclude=exclude or "",
	select=select,
	exclude=exclude if exclude is not None else "",

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

[dpeng817]

why are these set to two different values in different places? Use of empty string vs none feels inconsistent here. We should have a "default" constant that we reference everywhere; and it should probably only be set at the top level

[maximearmstrong]

Yeah I agree. I updated the code to use default constants. Since all these APIs are user-facing, I used the defaults in all of them, but not in the state back defs, where select and exclude must be passed by load_specs.

DBT_CLOUD_DEFAULT_SELECT = "fqn:*"
DBT_CLOUD_DEFAULT_EXCLUDE = ""

[dpeng817]

yea really don't like how this is optional at every layer down the chain - feels error prone

[dpeng817]

I want to see more consistency with how we're handling defaults here. I really don't like that they're optional all the way down the chain. We should have defaults at the top level and that's it.

[graphite-app]

There's a subtle issue with the conditional tag creation. When exclude is an empty string (the default value), the condition if exclude else {} evaluates to false, so the DAGSTER_DBT_EXCLUDE_METADATA_KEY tag won't be added to op_tags.

Later in the code, get_subset_selection_for_context() uses context.op.tags.get(DAGSTER_DBT_EXCLUDE_METADATA_KEY), which will return None instead of an empty string. This could cause inconsistent behavior if None and empty string are handled differently in the selection logic.

The tests expect the exclude tag to be set correctly (as shown in the assertions). Consider always including both tags:

op_tags = {
    DAGSTER_DBT_SELECT_METADATA_KEY: select,
    DAGSTER_DBT_EXCLUDE_METADATA_KEY: exclude,
}

This ensures consistent behavior regardless of whether the values are empty strings or not.

Suggested change

	op_tags = {
	**({DAGSTER_DBT_SELECT_METADATA_KEY: select} if select else {}),
	**({DAGSTER_DBT_EXCLUDE_METADATA_KEY: exclude} if exclude else {}),
	}
	op_tags = {
	DAGSTER_DBT_SELECT_METADATA_KEY: select,
	DAGSTER_DBT_EXCLUDE_METADATA_KEY: exclude,
	}

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

[dpeng817]

This is real. Pls heed

[maximearmstrong]

Done in e40d586

[dpeng817]

Specifically I think these arguments should not be optional in internal APIs

[maximearmstrong]

That's fair - updated in b3ada91