Updates to warehouse-per-model logging and tests #511
Conversation
Updated the debug logging for warehouse-per-model to include thread id and relation name. Updated the warehouse-per-model functional test to include a seed and a snapshot that specify an alternate compute. Changed the test conditions to include checking log output for messages that indicate the alternate compute was used. Signed-off-by: Raymond Cypher <[email protected]>
rcypher-databricks
requested review from
andrefurlan-db,
susodapop and
benc-db
as code owners
| @@ -1120,21 +1121,31 @@ def _get_compute_name(node: Optional[ResultNode]) -> Optional[str]: | |||
|
|
|||
|
|
|||
| def _get_http_path(node: Optional[ResultNode], creds: DatabricksCredentials) -> Optional[str]: | |||
| thread_id = (os.getpid(), get_ident()) | |||
| # Get the http path of the compute resource specified in the node's config. | |||
| # If none is specified return the default path from creds. | |||
| compute_name = _get_compute_name(node) | |||
| if not node or not compute_name: | |||
There was a problem hiding this comment.
This structure has become a little weird. I believe we can drop the if not node.
There was a problem hiding this comment.
maybe we can't due to the creds.http_path. Still, worth thinking about restructuring because its so odd to check if something is not, and then immediately check if it is.
There was a problem hiding this comment.
Given this weirdness, let's add either a unit test or an end to end test that validates we do the expected thing in the default compute case.
There was a problem hiding this comment.
I agree that if not node or not compute_name: is an odd structure. However I originally did it that way because using just if not compute_name: produced linting errors. I'll re-organize things.
There was a problem hiding this comment.
I think the existing unit tests cover all the possibilities for what can be passed in to _get_http_path
Updated functional tests to add a model using the default compute alongside models using alternate compute. Updated the error message for when a named compute does not exist or does not specify http_path. Clarified the logic in _get_http_path() Signed-off-by: Raymond Cypher <[email protected]>
Description
Checklist
CHANGELOG.mdand added information about my change to the "dbt-databricks next" section.