add initial support for semantic_manifest files #106
Conversation
WalkthroughThis change enhances the library's functionality by adding support for parsing semantic manifest files from DBT. It introduces a new function to handle JSON data structures specific to semantic manifests, expanding the parser's capabilities. Additionally, new data models are established for validation and organization, ensuring compliance and accuracy when handling these artifacts. Overall, this improvement aims to better integrate semantic artifacts into the existing framework. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
tests/test_parser.py (1)
136-155: Planned Test: Semantic Manifest ParsingThe commented-out test function
test_parse_semantic_manifestindicates a plan to test semantic manifest parsing. Ensure to implement and activate this test once the relevant schema details are finalized.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
tests/resources/v1/jaffle_shop/semantic_manifest.jsonis excluded by!**/*.json
Files selected for processing (7)
- README.md (1 hunks)
- dbt_artifacts_parser/parser.py (2 hunks)
- dbt_artifacts_parser/parsers/semantic_manifest/init.py (1 hunks)
- dbt_artifacts_parser/parsers/semantic_manifest/semantic_manifest_v1.py (1 hunks)
- dbt_artifacts_parser/parsers/version_map.py (2 hunks)
- tests/parsers/test_utils.py (2 hunks)
- tests/test_parser.py (1 hunks)
Files skipped from review due to trivial changes (1)
- dbt_artifacts_parser/parsers/semantic_manifest/init.py
Additional context used
Ruff
tests/parsers/test_utils.py
38-38:
dbt_artifacts_parser.parsers.semantic_manifest.semantic_manifest_v1.SemanticManifestV1imported but unusedRemove unused import:
dbt_artifacts_parser.parsers.semantic_manifest.semantic_manifest_v1.SemanticManifestV1(F401)
Additional comments not posted (14)
dbt_artifacts_parser/parsers/semantic_manifest/semantic_manifest_v1.py (9)
6-13: LGTM!The
NodeRelationclass is well-defined with appropriate fields and configuration.
16-22: LGTM!The
Measureclass is well-defined with appropriate fields and configuration.
25-36: LGTM!The
TypeParamsclass is well-structured with appropriate fields and default values.
39-48: LGTM!The
Metricclass is well-defined with appropriate fields and configuration.
51-57: LGTM!The
TimeSpineTableConfigurationclass is well-defined with appropriate fields and configuration.
60-66: LGTM!The
ProjectConfigurationclass is well-structured with appropriate fields and configuration.
69-75: LGTM!The
SavedQueryExportConfigclass is well-defined with appropriate fields and configuration.
95-104: LGTM!The
SavedQueryclass is well-structured with appropriate fields and configuration.
123-127: LGTM!The
SemanticManifestV1class is well-defined with appropriate fields and configuration.dbt_artifacts_parser/parsers/version_map.py (2)
41-41: LGTM!The import statement for
SemanticManifestV1is correctly added and necessary for the new functionality.
98-100: LGTM!The new enum entry
SEMANTIC_MANIFEST_V1is correctly added and consistent with existing patterns.tests/parsers/test_utils.py (1)
124-125: LGTM! Consider tracking this task.The commented-out line for
SemanticManifestV1indicates planned future work. Consider tracking this task in an issue if not already done.README.md (1)
240-251: Documentation Update: Semantic Manifest ParsingThe addition of the semantic manifest parsing example is clear and consistent with existing documentation. It effectively demonstrates the new functionality.
dbt_artifacts_parser/parser.py (1)
306-318: New Function:parse_semantic_manifestThe addition of
parse_semantic_manifestis well-structured and aligns with the existing parser functions. The placeholder for future schema version handling is a good practice for extensibility.
| @@ -35,6 +35,7 @@ | |||
| from dbt_artifacts_parser.parsers.run_results.run_results_v2 import RunResultsV2 | |||
| from dbt_artifacts_parser.parsers.run_results.run_results_v3 import RunResultsV3 | |||
| from dbt_artifacts_parser.parsers.run_results.run_results_v4 import RunResultsV4 | |||
| from dbt_artifacts_parser.parsers.semantic_manifest.semantic_manifest_v1 import SemanticManifestV1 | |||
There was a problem hiding this comment.
Remove unused import.
The import SemanticManifestV1 is currently unused and should be removed to address the static analysis warning.
- from dbt_artifacts_parser.parsers.semantic_manifest.semantic_manifest_v1 import SemanticManifestV1Committable 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.
| from dbt_artifacts_parser.parsers.semantic_manifest.semantic_manifest_v1 import SemanticManifestV1 |
Tools
Ruff
38-38:
dbt_artifacts_parser.parsers.semantic_manifest.semantic_manifest_v1.SemanticManifestV1imported but unusedRemove unused import:
dbt_artifacts_parser.parsers.semantic_manifest.semantic_manifest_v1.SemanticManifestV1(F401)
|
@MiConnell Thank you for opening this. I want to support |
|
@MiConnell Let me close this once. Please feel free to reopen it, when it gets ready. |
|
As far as I know, |
|
To be fair the example semantic manifest file on the docs site wasn't even up to date, I opened a PR to add missing fields so who knows what the actual schema is. |
|
It is an example of |
This PR begins the process for supporting semantic manifest files. I'd love your feedback on the logic I set up and if I missed anything. As of now, there's no semantic_manifest schema defined in dbt's schema site, but there's some discussion of that getting updated shortly.
Fixes #105