Importdbtproject #1077
Importdbtproject #1077
Conversation
|
""" WalkthroughA new utility module for importing dbt project metadata was added, along with a Django management command to load dbt manifest data into the database for a specific organization. Comprehensive unit tests for the import utilities were introduced. Minor import order adjustments were made in an unrelated schema file. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin (CLI)
participant Django as Django Management Command
participant ImportUtil as importdbtproject.py
participant DB as Database
Admin->>Django: Run github_to_ui4t_2 with org and manifest path
Django->>DB: Lookup Org and OrgDbt
Django->>ImportUtil: extract_models(manifest)
Django->>ImportUtil: extract_sources(manifest)
loop For each model
Django->>ImportUtil: create_orgdbtmodel_model(orgdbt, model_metadata)
Django->>ImportUtil: create_orgdbtedges(orgdbt, model_metadata)
Django->>ImportUtil: create_orgdbtoperation(orgdbt, model_metadata)
ImportUtil->>DB: Create OrgDbtModel, DbtEdge, OrgDbtOperation
end
loop For each source
Django->>ImportUtil: create_orgdbtmodel_source(orgdbt, source_metadata)
ImportUtil->>DB: Create OrgDbtModel
end
Django-->>Admin: Print completion message
""" 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1077 +/- ##
==========================================
+ Coverage 70.68% 70.80% +0.11%
==========================================
Files 86 87 +1
Lines 8109 8176 +67
==========================================
+ Hits 5732 5789 +57
- Misses 2377 2387 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
ddpui/utils/importdbtproject.py (3)
49-49: Simplify dictionary key iteration.Remove the unnecessary
.keys()call:-columns=[column_name for column_name in node_value.get("columns", {}).keys()], +columns=[column_name for column_name in node_value.get("columns", {})],
79-87: Combine nested if statements for better readability.Merge the conditions into a single if statement:
-if ( - model_metadata.name - and model_metadata.original_file_path - and model_metadata.dbschema - and model_metadata.resource_type -): - if not OrgDbtModel.objects.filter( - orgdbt=orgdbt, schema=model_metadata.dbschema, name=model_metadata.name - ).exists(): +if ( + model_metadata.name + and model_metadata.original_file_path + and model_metadata.dbschema + and model_metadata.resource_type + and not OrgDbtModel.objects.filter( + orgdbt=orgdbt, schema=model_metadata.dbschema, name=model_metadata.name + ).exists() +):
102-112: Combine nested if statements for consistency.Similar to the model creation function, merge the conditions:
-if ( - source_metadata.name - and source_metadata.dbschema - and source_metadata.database - and source_metadata.resource_type -): - if not OrgDbtModel.objects.filter( - orgdbt=orgdbt, - schema=source_metadata.dbschema, - name=source_metadata.identifier, - ).exists(): +if ( + source_metadata.name + and source_metadata.dbschema + and source_metadata.database + and source_metadata.resource_type + and not OrgDbtModel.objects.filter( + orgdbt=orgdbt, + schema=source_metadata.dbschema, + name=source_metadata.identifier, + ).exists() +):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ddpui/ddpdbt/schema.py(1 hunks)ddpui/management/commands/github_to_ui4t_2.py(1 hunks)ddpui/tests/utils/test_importdbtproject.py(1 hunks)ddpui/utils/importdbtproject.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
ddpui/management/commands/github_to_ui4t_2.py (2)
ddpui/models/org.py (2)
Org(61-83)OrgDbt(40-58)ddpui/utils/importdbtproject.py (6)
extract_models(34-53)create_orgdbtmodel_model(77-97)extract_sources(56-74)create_orgdbtmodel_source(100-122)create_orgdbtedges(125-143)create_orgdbtoperation(146-195)
ddpui/utils/importdbtproject.py (3)
ddpui/models/org.py (1)
OrgDbt(40-58)ddpui/models/dbt_workflow.py (3)
DbtEdge(68-81)OrgDbtModel(33-50)OrgDbtOperation(53-65)ddpui/tests/services/test_dbtautomation_service.py (2)
orgdbt(34-43)orgdbt_model(47-69)
🪛 Ruff (0.11.9)
ddpui/utils/importdbtproject.py
49-49: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
79-87: Use a single if statement instead of nested if statements
Combine if statements using and
(SIM102)
102-112: Use a single if statement instead of nested if statements
Combine if statements using and
(SIM102)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: checks (3.10, 6)
🔇 Additional comments (4)
ddpui/ddpdbt/schema.py (1)
1-3: LGTM! Import ordering follows convention.The reordering places standard library imports (
pathlib) before third-party imports (ninja), which follows Python import conventions.ddpui/utils/importdbtproject.py (2)
12-13: Ensure Python 3.10+ compatibility for union type syntax.The code uses the
str | Noneunion type syntax which requires Python 3.10+. Ensure your project's minimum Python version supports this syntax, otherwise useOptional[str]from thetypingmodule.Also applies to: 24-31
177-184: Hard-coded configuration values need clarification.The config contains placeholder values like
"op_config": "op_type"and"type": "new_op_type". These should either use actual values from the metadata or be documented as placeholders.Are these intended to be placeholder values? If so, consider adding a TODO comment to indicate they need to be replaced with actual implementation.
ddpui/tests/utils/test_importdbtproject.py (1)
1-536: Excellent test coverage!The test suite is comprehensive and well-structured:
- Good coverage of edge cases (empty manifests, missing fields)
- Proper use of mocking for Django ORM interactions
- Clear test names and documentation
- Tests for both success and failure scenarios
| for dependency in model_metadata.depends_on: | ||
| dependency: str | ||
| if dependency.startswith("source."): | ||
| _, _, _, dep_name = dependency.split(".", 3) | ||
| parent_model = OrgDbtModel.objects.get(display_name=dep_name, type="source") | ||
|
|
||
| elif dependency.startswith("model."): | ||
| _, _, dep_name = dependency.split(".", 2) | ||
| parent_model = OrgDbtModel.objects.get(name=dep_name, type="model") | ||
|
|
||
| else: | ||
| continue | ||
|
|
There was a problem hiding this comment.
Logic issue: Only the last dependency is used.
The loop iterates through all dependencies but only uses the last parent_model found. If a model has multiple dependencies, only the last one will be included in the operation config.
Consider collecting all parent models or clarifying the intended behavior with a comment if only one parent is expected.
🤖 Prompt for AI Agents
In ddpui/utils/importdbtproject.py around lines 153 to 165, the loop over
model_metadata.depends_on assigns parent_model repeatedly, so only the last
dependency's parent_model is used. To fix this, collect all parent models in a
list during the loop instead of overwriting parent_model each time, then use
this list in the subsequent operation config. If only one parent is intended,
add a comment clarifying this behavior.
| parent_model = None | ||
| for dependency in model_metadata.depends_on: | ||
| dependency: str | ||
| if dependency.startswith("source."): | ||
| _, _, _, dep_name = dependency.split(".", 3) | ||
| parent_model = OrgDbtModel.objects.get(display_name=dep_name, type="source") | ||
|
|
||
| elif dependency.startswith("model."): | ||
| _, _, dep_name = dependency.split(".", 2) | ||
| parent_model = OrgDbtModel.objects.get(name=dep_name, type="model") | ||
|
|
||
| else: | ||
| continue | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract duplicated dependency resolution logic.
This code is duplicated from create_orgdbtedges. Consider extracting it into a helper function to follow DRY principles.
Would you like me to create a helper function to resolve dependencies that can be used by both create_orgdbtedges and create_orgdbtoperation?
🤖 Prompt for AI Agents
In ddpui/utils/importdbtproject.py around lines 152 to 165, the logic for
resolving dependencies from model metadata is duplicated from the
create_orgdbtedges function. Extract this dependency resolution code into a
separate helper function that takes a dependency string and returns the
corresponding OrgDbtModel instance. Then update both create_orgdbtedges and
create_orgdbtoperation to call this new helper function instead of duplicating
the logic, ensuring DRY principles are followed.
ddpui/utils/importdbtproject.py
Outdated
| name=model_metadata.name, | ||
| sql_path=model_metadata.original_file_path, | ||
| type=model_metadata.resource_type, | ||
| source_name=model_metadata.dbschema, |
There was a problem hiding this comment.
Potential bug: source_name should use model's source_name, not schema.
The source_name field is being set to model_metadata.dbschema, but it should likely be model_metadata.source_name to maintain consistency with the metadata structure.
Apply this fix:
-source_name=model_metadata.dbschema,
+source_name=model_metadata.source_name,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| source_name=model_metadata.dbschema, | |
| source_name=model_metadata.source_name, |
🤖 Prompt for AI Agents
In ddpui/utils/importdbtproject.py at line 95, the source_name is incorrectly
assigned from model_metadata.dbschema. Change this assignment to use
model_metadata.source_name instead to correctly reflect the model's source name
in the metadata.
| orgdbt_model = OrgDbtModel.objects.get( | ||
| orgdbt=orgdbt, schema=model_metadata.dbschema, name=model_metadata.name | ||
| ) | ||
| for dependency in model_metadata.depends_on: | ||
| dependency: str | ||
| if dependency.startswith("source."): | ||
| _, _, _, dep_name = dependency.split(".", 3) | ||
| parent_model = OrgDbtModel.objects.get(display_name=dep_name, type="source") | ||
|
|
||
| elif dependency.startswith("model."): | ||
| _, _, dep_name = dependency.split(".", 2) | ||
| parent_model = OrgDbtModel.objects.get(name=dep_name, type="model") | ||
|
|
||
| else: | ||
| continue | ||
|
|
||
| DbtEdge.objects.create(from_node=parent_model, to_node=orgdbt_model) |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Add error handling for missing models.
The function can raise DoesNotExist exceptions if models are not found. Consider adding try-except blocks or using .filter().first() pattern.
Consider this safer approach:
orgdbt_model = OrgDbtModel.objects.filter(
orgdbt=orgdbt, schema=model_metadata.dbschema, name=model_metadata.name
).first()
if not orgdbt_model:
print(f"Model not found: {model_metadata.dbschema}.{model_metadata.name}")
return
for dependency in model_metadata.depends_on:
parent_model = None
if dependency.startswith("source."):
_, _, _, dep_name = dependency.split(".", 3)
parent_model = OrgDbtModel.objects.filter(display_name=dep_name, type="source").first()
elif dependency.startswith("model."):
_, _, dep_name = dependency.split(".", 2)
parent_model = OrgDbtModel.objects.filter(name=dep_name, type="model").first()
if parent_model:
DbtEdge.objects.create(from_node=parent_model, to_node=orgdbt_model)
else:
print(f"Parent model not found for dependency: {dependency}")Add error handling around OrgDbtModel lookups
The calls to OrgDbtModel.objects.get(…) (lines 127–129, 134, 138 in ddpui/utils/importdbtproject.py) will raise OrgDbtModel.DoesNotExist if no record is found. To prevent unhandled exceptions, wrap these lookups in try/except blocks or switch to a safe lookup using .filter().first():
• Lines 127–129: guard against missing current model
• Lines 134 & 138: guard against missing parent models
For example:
- orgdbt_model = OrgDbtModel.objects.get(
- orgdbt=orgdbt, schema=model_metadata.dbschema, name=model_metadata.name
- )
+ orgdbt_model = OrgDbtModel.objects.filter(
+ orgdbt=orgdbt, schema=model_metadata.dbschema, name=model_metadata.name
+ ).first()
+ if not orgdbt_model:
+ logger.warning(f"Model not found: {model_metadata.dbschema}.{model_metadata.name}")
+ return
...
- parent_model = OrgDbtModel.objects.get(display_name=dep_name, type="source")
+ parent_model = OrgDbtModel.objects.filter(
+ display_name=dep_name, type="source"
+ ).first()
+ if not parent_model:
+ logger.warning(f"Source parent not found for dependency: {dependency}")
+ continueThis ensures the function handles missing records gracefully instead of crashing.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| orgdbt_model = OrgDbtModel.objects.get( | |
| orgdbt=orgdbt, schema=model_metadata.dbschema, name=model_metadata.name | |
| ) | |
| for dependency in model_metadata.depends_on: | |
| dependency: str | |
| if dependency.startswith("source."): | |
| _, _, _, dep_name = dependency.split(".", 3) | |
| parent_model = OrgDbtModel.objects.get(display_name=dep_name, type="source") | |
| elif dependency.startswith("model."): | |
| _, _, dep_name = dependency.split(".", 2) | |
| parent_model = OrgDbtModel.objects.get(name=dep_name, type="model") | |
| else: | |
| continue | |
| DbtEdge.objects.create(from_node=parent_model, to_node=orgdbt_model) | |
| orgdbt_model = OrgDbtModel.objects.filter( | |
| orgdbt=orgdbt, schema=model_metadata.dbschema, name=model_metadata.name | |
| ).first() | |
| if not orgdbt_model: | |
| logger.warning(f"Model not found: {model_metadata.dbschema}.{model_metadata.name}") | |
| return | |
| for dependency in model_metadata.depends_on: | |
| dependency: str | |
| if dependency.startswith("source."): | |
| _, _, _, dep_name = dependency.split(".", 3) | |
| parent_model = OrgDbtModel.objects.filter( | |
| display_name=dep_name, type="source" | |
| ).first() | |
| if not parent_model: | |
| logger.warning(f"Source parent not found for dependency: {dependency}") | |
| continue | |
| elif dependency.startswith("model."): | |
| _, _, dep_name = dependency.split(".", 2) | |
| parent_model = OrgDbtModel.objects.get(name=dep_name, type="model") | |
| else: | |
| continue | |
| DbtEdge.objects.create(from_node=parent_model, to_node=orgdbt_model) |
🤖 Prompt for AI Agents
In ddpui/utils/importdbtproject.py around lines 127 to 143, the calls to
OrgDbtModel.objects.get() can raise OrgDbtModel.DoesNotExist exceptions if no
matching record is found. To fix this, wrap each get() call in a try/except
block catching DoesNotExist or replace them with a filter().first() call to
safely return None if no record exists. Then add logic to handle the None case
appropriately to avoid unhandled exceptions and ensure the function continues
gracefully.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
ddpui/tests/utils/test_importdbtproject.py (1)
535-535: Missing change marker on line 535.Line 535 is missing the
~marker that indicates it's part of the changes. Since this is a new file, all lines should be marked with~.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ddpui/management/commands/github_to_ui4t_2.py(1 hunks)ddpui/tests/utils/test_importdbtproject.py(1 hunks)ddpui/utils/importdbtproject.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- ddpui/management/commands/github_to_ui4t_2.py
- ddpui/utils/importdbtproject.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
ddpui/tests/utils/test_importdbtproject.py (1)
ddpui/utils/importdbtproject.py (8)
ModelMetadata(9-18)SourceMetaData(21-31)extract_models(34-53)extract_sources(56-73)create_orgdbtmodel_model(76-96)create_orgdbtmodel_source(99-121)create_orgdbtedges(124-142)create_orgdbtoperation(145-194)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: checks (3.10, 6)
| class TestCreateOrgDbtOperation: | ||
| """Test create_orgdbtoperation function""" | ||
|
|
||
| @patch("ddpui.utils.importdbtproject.OrgDbtOperation") | ||
| @patch("ddpui.utils.importdbtproject.OrgDbtModel") | ||
| @patch("ddpui.utils.importdbtproject.uuid") | ||
| @patch("builtins.print") | ||
| def test_create_orgdbtoperation_source_dependency( | ||
| self, mock_print, mock_uuid, mock_orgdbtmodel, mock_orgdbtoperation | ||
| ): | ||
| """Test creating operation with source dependency""" | ||
| from ddpui.utils.importdbtproject import create_orgdbtoperation | ||
|
|
||
| mock_uuid.uuid4.return_value = "test-uuid" | ||
| mock_orgdbt = Mock() | ||
| mock_child_model = Mock() | ||
| mock_parent_model = Mock() | ||
| mock_parent_model.uuid = "parent-uuid" | ||
| mock_parent_model.name = "table1" | ||
| mock_parent_model.source_name = "test_source" | ||
| mock_parent_model.schema = "raw_data" | ||
| mock_parent_model.type = "source" | ||
|
|
||
| # Setup model queries | ||
| mock_orgdbtmodel.objects.get.side_effect = [mock_child_model, mock_parent_model] | ||
| mock_orgdbtoperation.objects.filter.return_value.exists.return_value = False | ||
|
|
||
| model_metadata = ModelMetadata( | ||
| name="test_model", | ||
| dbschema="test_schema", | ||
| original_file_path="models/test_model.sql", | ||
| resource_type="model", | ||
| source_name="test_package", | ||
| depends_on=["source.test_package.test_source.table1"], | ||
| columns=["col1", "col2"], | ||
| ) | ||
|
|
||
| create_orgdbtoperation(mock_orgdbt, model_metadata) | ||
|
|
||
| # Verify operation creation | ||
| mock_orgdbtoperation.objects.create.assert_called_once() | ||
| call_args = mock_orgdbtoperation.objects.create.call_args[1] | ||
| assert call_args["dbtmodel"] == mock_child_model | ||
| assert call_args["uuid"] == "test-uuid" | ||
| assert call_args["seq"] == 1 | ||
| assert call_args["output_cols"] == ["col1", "col2"] | ||
| assert "config" in call_args | ||
| assert call_args["config"]["input_models"][0]["uuid"] == "parent-uuid" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add missing test cases and verify print output.
The TestCreateOrgDbtOperation class needs additional coverage:
- Test for model dependencies (when dependency starts with "model.")
- Test for when no parent model is found (should trigger print statements)
- Test for when operation already exists
- The
mock_printis defined but never asserted - use it to verify the print output when no parent model is found
🤖 Prompt for AI Agents
In ddpui/tests/utils/test_importdbtproject.py around lines 465 to 513, the
TestCreateOrgDbtOperation class lacks coverage for model dependencies, missing
parent models, and existing operations, and does not verify print output. Add
test cases to cover dependencies starting with "model.", handle cases where no
parent model is found by asserting that the print function is called with the
expected message, and test the scenario where the operation already exists to
ensure correct behavior. Use the mock_print mock to verify print outputs in
relevant tests.
| class TestCreateOrgDbtEdges: | ||
| """Test create_orgdbtedges function""" | ||
|
|
||
| @patch("ddpui.utils.importdbtproject.DbtEdge") | ||
| @patch("ddpui.utils.importdbtproject.OrgDbtModel") | ||
| def test_create_orgdbtedges_source_dependency(self, mock_orgdbtmodel, mock_dbtedge): | ||
| """Test creating edges for source dependencies""" | ||
| from ddpui.utils.importdbtproject import create_orgdbtedges | ||
|
|
||
| mock_orgdbt = Mock() | ||
| mock_child_model = Mock() | ||
| mock_parent_model = Mock() | ||
|
|
||
| # Setup model queries | ||
| mock_orgdbtmodel.objects.get.side_effect = [mock_child_model, mock_parent_model] | ||
|
|
||
| model_metadata = ModelMetadata( | ||
| name="test_model", | ||
| dbschema="test_schema", | ||
| original_file_path="models/test_model.sql", | ||
| resource_type="model", | ||
| source_name="test_package", | ||
| depends_on=["source.test_package.test_source.table1"], | ||
| columns=["col1", "col2"], | ||
| ) | ||
|
|
||
| create_orgdbtedges(mock_orgdbt, model_metadata) | ||
|
|
||
| # Verify model queries | ||
| assert mock_orgdbtmodel.objects.get.call_count == 2 | ||
| mock_orgdbtmodel.objects.get.assert_any_call( | ||
| orgdbt=mock_orgdbt, schema="test_schema", name="test_model" | ||
| ) | ||
| mock_orgdbtmodel.objects.get.assert_any_call(display_name="table1", type="source") | ||
|
|
||
| # Verify edge creation | ||
| mock_dbtedge.objects.create.assert_called_once_with( | ||
| from_node=mock_parent_model, to_node=mock_child_model | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add test cases for model dependencies and edge cases.
The TestCreateOrgDbtEdges class only tests source dependencies. Please add:
- Test for model dependencies (when dependency starts with "model.")
- Test for empty
depends_onlist - Test for unsupported dependency types (neither "source." nor "model.")
🤖 Prompt for AI Agents
In ddpui/tests/utils/test_importdbtproject.py around lines 424 to 463, the
TestCreateOrgDbtEdges class currently only tests source dependencies. Add three
new test methods: one to test model dependencies where depends_on entries start
with "model.", another to test the behavior when depends_on is an empty list,
and a third to test how unsupported dependency types (not starting with
"source." or "model.") are handled. Each test should mock necessary objects,
call create_orgdbtedges with appropriate ModelMetadata, and assert expected
calls and edge creations or lack thereof.
| class TestCreateOrgDbtModelSource: | ||
| """Test create_orgdbtmodel_source function""" | ||
|
|
||
| @patch("ddpui.utils.importdbtproject.OrgDbtModel") | ||
| @patch("ddpui.utils.importdbtproject.uuid") | ||
| def test_create_orgdbtmodel_source_success(self, mock_uuid, mock_orgdbtmodel): | ||
| """Test successful creation of OrgDbtModel from source metadata""" | ||
| from ddpui.utils.importdbtproject import create_orgdbtmodel_source | ||
|
|
||
| # Setup mocks | ||
| mock_uuid.uuid4.return_value = "test-uuid" | ||
| mock_orgdbt = Mock() | ||
| mock_orgdbtmodel.objects.filter.return_value.exists.return_value = False | ||
|
|
||
| source_metadata = SourceMetaData( | ||
| name="test_source", | ||
| identifier="test_table", | ||
| dbschema="raw_data", | ||
| database="test_db", | ||
| resource_type="source", | ||
| package_name="test_package", | ||
| source_name="test_source_name", | ||
| path="models/sources.yml", | ||
| ) | ||
|
|
||
| create_orgdbtmodel_source(mock_orgdbt, source_metadata) | ||
|
|
||
| # Verify filter was called to check existence | ||
| mock_orgdbtmodel.objects.filter.assert_called_once_with( | ||
| orgdbt=mock_orgdbt, schema="raw_data", name="test_table" | ||
| ) | ||
|
|
||
| # Verify create was called | ||
| mock_orgdbtmodel.objects.create.assert_called_once_with( | ||
| uuid="test-uuid", | ||
| orgdbt=mock_orgdbt, | ||
| schema="raw_data", | ||
| name="test_table", | ||
| display_name="test_source", | ||
| type="source", | ||
| source_name="test_source_name", | ||
| sql_path="models/sources.yml", | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add missing test cases for better coverage.
The TestCreateOrgDbtModelSource class only tests the success case. For consistency with TestCreateOrgDbtModelModel and to ensure complete coverage, please add:
- Test for when source already exists (similar to
test_create_orgdbtmodel_model_already_exists) - Test for missing required fields (similar to
test_create_orgdbtmodel_model_missing_required_fields)
🤖 Prompt for AI Agents
In ddpui/tests/utils/test_importdbtproject.py around lines 379 to 422, the
TestCreateOrgDbtModelSource class only tests the success scenario. To improve
coverage, add two new test methods: one that mocks the filter().exists() call to
return True and verifies that no create call is made (testing the case when the
source already exists), and another that passes incomplete or missing required
fields in the source metadata and asserts that the function raises the
appropriate exception or handles the error as expected (testing missing required
fields).
handle errors from reading the manifest
|
@Ishankoradia shall we merge this? just to have the code in the main branch |
1 participant
Summary by CodeRabbit
New Features
Tests
Style