Update Manifest v12 json schema #130
Conversation
WalkthroughThe pull request introduces significant modifications to the Changes
Possibly related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
dbt_artifacts_parser/resources/manifest/manifest_v12.jsonis excluded by!**/*.json
📒 Files selected for processing (1)
dbt_artifacts_parser/parsers/manifest/manifest_v12.py(14 hunks)
🔇 Additional comments (13)
dbt_artifacts_parser/parsers/manifest/manifest_v12.py (13)
3226-3227: Ensure consistency in 'offset_window' and 'offset_to_grain' attributes
The offset_window and offset_to_grain attributes have been updated to Optional[OffsetWindow] and Optional[str]. Verify that all code interacting with these attributes in this class is updated accordingly.
3244-3245: Ensure consistency in 'offset_window' and 'offset_to_grain' attributes
The offset_window and offset_to_grain attributes have been updated similarly in this section. Please refer to the earlier comments for verification steps.
3270-3271: Ensure consistency in 'offset_window' and 'offset_to_grain' attributes
These attributes have undergone the same type changes. Ensure all related code is updated accordingly.
3397-3397: Adjust code for 'time_granularity' type change in Disabled10 class
The time_granularity attribute in the Disabled10 class is now Optional[str]. Ensure that any code utilizing this attribute handles the new type appropriately.
Run the following script to find usages of Disabled10.time_granularity:
1777-1778: Review changes in CumulativeTypeParams class attributes
The window attribute now uses Optional[Window1], and grain_to_date is updated to Optional[str]. Ensure that the codebase correctly handles these updated types wherever CumulativeTypeParams is utilized.
Run the following script to find usages of CumulativeTypeParams attributes:
3334-3335: Update 'CumulativeTypeParams1' class attributes
The window attribute is updated to Optional[Window4], and grain_to_date is Optional[str]. Verify that these changes are accommodated in all relevant parts of the code.
Run the following script to find usages of CumulativeTypeParams1 attributes:
1744-1744: Ensure 'granularity' type consistency in Window1 class
The granularity attribute in the Window1 class has been changed to type str. Please verify that all instances where Window1 is used are updated accordingly and that any related logic is adjusted.
Run the following script to locate usages of Window1.granularity:
1654-1655: Confirm updates to 'Denominator' class attributes
The offset_window and offset_to_grain attributes in the Denominator class have been updated to Optional[OffsetWindow] and Optional[str] respectively. Ensure that any code utilizing these attributes is updated to handle the new types.
Run the following script to check references to Denominator class attributes:
1637-1637: Verify the type change of 'offset_to_grain' in the Numerator class
The offset_to_grain attribute in the Numerator class is now of type Optional[str]. Please ensure that this change is consistently reflected throughout the codebase and that any logic depending on this attribute is updated accordingly.
Run the following script to locate usages of offset_to_grain:
✅ Verification successful
Let me gather more information about the type definition and usage.
Let me try one more verification to understand the type changes across versions.
Type change in offset_to_grain is an intentional API evolution
The change from Optional[OffsetToGrain] (an enum) to Optional[str] in manifest v12 appears to be an intentional API evolution. Previous versions (v10, v11) defined OffsetToGrain as an enum with a 'day' value, while v12 simplifies this to use strings directly. This change is consistently applied in both Numerator and Numerator1 classes within the same file.
manifest_v10.py: UsesOptional[OffsetToGrain]with enummanifest_v11.py: UsesOptional[OffsetToGrain]with enummanifest_v12.py: UsesOptional[str]in both Numerator classes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of 'offset_to_grain' outside the updated file.
# Search for 'offset_to_grain' in all Python files, excluding the current file.
rg --type py 'offset_to_grain' --glob '!dbt_artifacts_parser/parsers/manifest/manifest_v12.py'
Length of output: 306
Script:
#!/bin/bash
# Check the definition and usage of OffsetToGrain type and its context
ast-grep --pattern 'class $_ {
$$$
offset_to_grain: $_
$$$
}'
# Also check for any direct string assignments or comparisons
rg --type py "offset_to_grain.*=.*['\"]" -A 2
Length of output: 119
Script:
#!/bin/bash
# Check imports and type definitions in all manifest versions
rg --type py "^from.*OffsetToGrain|^class.*OffsetToGrain|type.*OffsetToGrain" -B 1 -A 1
# Check the Numerator class definition in v12
rg --type py "class Numerator" -B 2 -A 10 dbt_artifacts_parser/parsers/manifest/manifest_v12.py
Length of output: 1153
1850-1850: Adjust code for 'time_granularity' type change in Metrics class
The time_granularity attribute in the Metrics class is now Optional[str]. Verify that all usages of this attribute are updated to handle the new type, and update any validations or dependent code as necessary.
Run the following script to find usages of Metrics.time_granularity:
1694-1695: Update usages of 'Metric' class attributes
The offset_window and offset_to_grain attributes in the Metric class are now Optional[OffsetWindow] and Optional[str]. Verify that all code interacting with these attributes accommodates the type changes.
Run the following script to find usages of Metric class attributes:
✅ Verification successful
Type changes in Metric class are consistent with previous versions
The type changes for offset_window and offset_to_grain in manifest_v12.py are actually maintaining consistency with previous manifest versions (v10 and v11), where these attributes were already defined as Optional types. The only difference is the type of offset_window changing from MetricTimeWindow to OffsetWindow, and offset_to_grain from OffsetToGrain to str, which appears to be a deliberate type refinement.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find usages of 'Metric' class attributes outside the updated file.
# Search for references to 'Metric.offset_window' and 'Metric.offset_to_grain'.
rg --type py 'Metric\.(offset_window|offset_to_grain)' --glob '!dbt_artifacts_parser/parsers/manifest/manifest_v12.py'
Length of output: 120
Script:
#!/bin/bash
# Let's try a broader search to find any references to these attributes
# First, let's find all Python files that mention 'offset_window' or 'offset_to_grain'
rg --type py -l "(offset_window|offset_to_grain)"
# Then, let's also look for any usage of Metric class to understand the context
rg --type py -l "class Metric"
Length of output: 726
Script:
#!/bin/bash
# Let's examine how these attributes are used in v10 and v11 manifest files
rg -A 2 -B 2 "(offset_window|offset_to_grain)" dbt_artifacts_parser/parsers/manifest/manifest_v10.py dbt_artifacts_parser/parsers/manifest/manifest_v11.py
# Also let's check if there are any type hints for these attributes in previous versions
ast-grep --pattern 'class Metric:
$$$
offset_window: $_
$$$'
Length of output: 1248
1626-1626: Ensure consistent handling of 'granularity' attribute
The granularity attribute in the OffsetWindow class has been changed to type str from the Granularity enum. Ensure that all references to this attribute across the codebase are updated accordingly, and that any enum-based validations or constraints are appropriately adjusted.
Run the following script to identify usages of the granularity attribute:
3315-3315: Verify 'granularity' type change in Window4 class
The granularity attribute in the Window4 class is now of type str. Ensure that this change is propagated throughout the codebase where Window4 is utilized.
Run the following script to find usages of Window4.granularity:
| class TimeGranularity(Enum): | ||
| nanosecond = 'nanosecond' | ||
| microsecond = 'microsecond' | ||
| millisecond = 'millisecond' | ||
| second = 'second' | ||
| minute = 'minute' | ||
| hour = 'hour' | ||
| day = 'day' | ||
| week = 'week' | ||
| month = 'month' | ||
| quarter = 'quarter' | ||
| year = 'year' | ||
|
|
||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using 'TimeGranularity' enum for 'granularity' attributes
The TimeGranularity enum is now defined, providing a set of valid time granularities. To enhance type safety and maintain consistency, consider updating the granularity attributes in the respective classes to use TimeGranularity instead of str.
Apply this diff to update the OffsetWindow class:
class OffsetWindow(BaseParserModel):
count: int
- granularity: str
+ granularity: TimeGranularityRepeat similar updates for other classes where granularity is currently typed as str.
Committable suggestion skipped: line range outside the PR's diff.
|
This change has been released in v0.8.0. |
User description
Current json schema does not match the original v12 schema https://schemas.getdbt.com/dbt/manifest/v12.json
hence when parsing manifest file in some cases where
offset_to_grainis not one of the previous enums it fails to parse.This pr updates the json schema and updates the model.
PR Type
Bug fix
Description
Changes walkthrough 📝
manifest_v12.py
Refactor time granularity fields to accept string valuesdbt_artifacts_parser/parsers/manifest/manifest_v12.py
offset_to_grain, time_granularity)
manifest_v12.json
Update JSON schema to support flexible time granularity valuesdbt_artifacts_parser/resources/manifest/manifest_v12.json
fields
time_granularity properties