-
Notifications
You must be signed in to change notification settings - Fork 8
refactor: prior to introducing planner #586
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe pull request removes the deprecated DataModel enumeration and multiple orchestration/repository files while updating naming conventions across the codebase. References to lowercase “Entities” and “Events” are now replaced with uppercase “ENTITIES” and “EVENTS.” In addition, a new configuration-and-node conversion framework is introduced (ConfToNode.scala), package declarations and import statements are restructured, and Thrift definitions are updated with field reordering and type changes. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant F as Config Loader
participant N as Node Builder
C->>F: Call from(filePath)
F->>N: Build assets & nodes
N-->>F: Return node configuration
F-->>C: Provide Conf instance
sequenceDiagram
participant U as User
participant J as Join Handler
participant JU as JoinUtils
U->>J: Initiate join request
J->>JU: Compute left source table name
JU->>JU: Validate using ENTITIES/(EVENTS)
JU-->>J: Return full table name
J-->>U: Complete join process
Possibly related PRs
Poem
Warning Review ran into problems🔥 ProblemsGitHub Actions and Pipeline Checks: Resource not accessible by integration - https://docs.github.com/rest/actions/workflow-runs#list-workflow-runs-for-a-repository. Please grant the required permissions to the CodeRabbit GitHub App under the organization or repository settings. 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (9)
api/src/main/scala/ai/chronon/api/Extensions.scala (1)
1213-1214: Function signature updated with unused parameterA
joinOutputTableparameter was added but not used in the method.Consider removing the unused parameter or utilizing it if it's intended for future functionality.
api/src/main/scala/ai/chronon/api/planner/ConfToNode.scala (8)
1-10: Suggest adding brief documentation.
These lines look fine, but docstrings or scaladocs onAssetcould clarify usage.
33-35: Implementation missing inConf.from(...).
This method is unimplemented; fill it or remove if unused.
37-134: Methods are just placeholders.
All node case classes have???; ensure correct logic is added before production.
220-220:onlineNodesnot implemented.
Fill in online node generation or confirm if it's unnecessary.
223-241: Staging and GroupBy conf incomplete.
Both offline/online methods are???; clarify final usage or fill logic.
244-252:ModelConfincomplete.
Offline and online node methods are???; implement if required.
255-261: EmptyDependencydefinitions.
Trait/Object have no members; remove or expand as needed.
270-272:ConfToNodesobject is empty.
Confirm its purpose or remove if it’s not needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (53)
api/src/main/scala/ai/chronon/api/DataModel.scala(0 hunks)api/src/main/scala/ai/chronon/api/Extensions.scala(6 hunks)api/src/main/scala/ai/chronon/api/planner/ConfToNode.scala(1 hunks)api/src/main/scala/ai/chronon/api/planner/DependencyResolver.scala(1 hunks)api/src/main/scala/ai/chronon/api/planner/RelevantLeftForJoinPart.scala(2 hunks)api/src/test/scala/ai/chronon/api/test/RelevantLeftForJoinPartSpec.scala(1 hunks)api/thrift/api.thrift(1 hunks)api/thrift/orchestration.thrift(2 hunks)flink/src/main/scala/ai/chronon/flink/AvroCodecFn.scala(2 hunks)online/src/main/scala/ai/chronon/online/DataStreamBuilder.scala(3 hunks)online/src/main/scala/ai/chronon/online/GroupByServingInfoParsed.scala(1 hunks)online/src/main/scala/ai/chronon/online/fetcher/GroupByResponseHandler.scala(2 hunks)online/src/test/scala/ai/chronon/online/test/DataStreamBuilderTest.scala(1 hunks)orchestration/src/main/scala/ai/chronon/orchestration/RepoIndex.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/RepoNodes.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/RepoParser.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/RepoTypes.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/VersionUpdate.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/logical/GroupByNodeImpl.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/logical/JoinNodeImpl.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/logical/LogicalNodeImpl.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/logical/ModelNodeImpl.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/logical/StagingQueryNodeImpl.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/logical/TabularDataNodeImpl.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/physical/ComputeNodeRunner.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/physical/GroupByBackfill.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/physical/GroupByPrepareUpload.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/physical/JoinBackfill.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/physical/LabelJoin.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/physical/StagingQueryNode.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/physical/TabularNode.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/utils/Config.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/utils/SequenceMap.scala(0 hunks)orchestration/src/test/scala/ai/chronon/orchestration/test/RepoIndexSpec.scala(0 hunks)orchestration/src/test/scala/ai/chronon/orchestration/test/SequenceMapSpec.scala(0 hunks)spark/src/main/scala/ai/chronon/spark/Analyzer.scala(2 hunks)spark/src/main/scala/ai/chronon/spark/BootstrapJob.scala(1 hunks)spark/src/main/scala/ai/chronon/spark/GroupBy.scala(5 hunks)spark/src/main/scala/ai/chronon/spark/GroupByUpload.scala(1 hunks)spark/src/main/scala/ai/chronon/spark/Join.scala(5 hunks)spark/src/main/scala/ai/chronon/spark/JoinBase.scala(2 hunks)spark/src/main/scala/ai/chronon/spark/JoinDerivationJob.scala(1 hunks)spark/src/main/scala/ai/chronon/spark/JoinPartJob.scala(5 hunks)spark/src/main/scala/ai/chronon/spark/JoinUtils.scala(6 hunks)spark/src/main/scala/ai/chronon/spark/LabelJoin.scala(4 hunks)spark/src/main/scala/ai/chronon/spark/MergeJob.scala(4 hunks)spark/src/main/scala/ai/chronon/spark/SourceJob.scala(3 hunks)spark/src/main/scala/ai/chronon/spark/stats/CompareJob.scala(2 hunks)spark/src/main/scala/ai/chronon/spark/streaming/GroupBy.scala(4 hunks)spark/src/main/scala/ai/chronon/spark/streaming/JoinSourceRunner.scala(1 hunks)spark/src/test/scala/ai/chronon/spark/test/OnlineUtils.scala(1 hunks)spark/src/test/scala/ai/chronon/spark/test/join/JoinTest.scala(1 hunks)spark/src/test/scala/ai/chronon/spark/test/join/ModularJoinTest.scala(5 hunks)
💤 Files with no reviewable changes (23)
- orchestration/src/main/scala/ai/chronon/orchestration/physical/LabelJoin.scala
- orchestration/src/main/scala/ai/chronon/orchestration/logical/LogicalNodeImpl.scala
- orchestration/src/main/scala/ai/chronon/orchestration/logical/GroupByNodeImpl.scala
- orchestration/src/main/scala/ai/chronon/orchestration/logical/TabularDataNodeImpl.scala
- orchestration/src/main/scala/ai/chronon/orchestration/physical/GroupByBackfill.scala
- orchestration/src/main/scala/ai/chronon/orchestration/RepoParser.scala
- orchestration/src/test/scala/ai/chronon/orchestration/test/SequenceMapSpec.scala
- orchestration/src/main/scala/ai/chronon/orchestration/physical/GroupByPrepareUpload.scala
- api/src/main/scala/ai/chronon/api/DataModel.scala
- orchestration/src/main/scala/ai/chronon/orchestration/physical/ComputeNodeRunner.scala
- orchestration/src/main/scala/ai/chronon/orchestration/RepoNodes.scala
- orchestration/src/main/scala/ai/chronon/orchestration/physical/TabularNode.scala
- orchestration/src/main/scala/ai/chronon/orchestration/logical/ModelNodeImpl.scala
- orchestration/src/main/scala/ai/chronon/orchestration/physical/StagingQueryNode.scala
- orchestration/src/main/scala/ai/chronon/orchestration/logical/JoinNodeImpl.scala
- orchestration/src/main/scala/ai/chronon/orchestration/physical/JoinBackfill.scala
- orchestration/src/main/scala/ai/chronon/orchestration/utils/Config.scala
- orchestration/src/main/scala/ai/chronon/orchestration/logical/StagingQueryNodeImpl.scala
- orchestration/src/main/scala/ai/chronon/orchestration/RepoIndex.scala
- orchestration/src/test/scala/ai/chronon/orchestration/test/RepoIndexSpec.scala
- orchestration/src/main/scala/ai/chronon/orchestration/utils/SequenceMap.scala
- orchestration/src/main/scala/ai/chronon/orchestration/VersionUpdate.scala
- orchestration/src/main/scala/ai/chronon/orchestration/RepoTypes.scala
🧰 Additional context used
🧬 Code Definitions (22)
spark/src/main/scala/ai/chronon/spark/streaming/JoinSourceRunner.scala (2)
online/src/main/scala/ai/chronon/online/GroupByServingInfoParsed.scala (1)
mutationValueChrononSchema(122-124)api/src/main/scala/ai/chronon/api/Extensions.scala (2)
dataModel(320-325)dataModel(456-463)
api/src/test/scala/ai/chronon/api/test/RelevantLeftForJoinPartSpec.scala (1)
api/src/main/scala/ai/chronon/api/planner/RelevantLeftForJoinPart.scala (2)
RelevantLeftForJoinPart(15-23)RelevantLeftForJoinPart(25-91)
online/src/test/scala/ai/chronon/online/test/DataStreamBuilderTest.scala (1)
online/src/main/scala/ai/chronon/online/DataStreamBuilder.scala (2)
DataStream(54-106)apply(58-105)
spark/src/main/scala/ai/chronon/spark/BootstrapJob.scala (1)
spark/src/main/scala/ai/chronon/spark/JoinUtils.scala (2)
JoinUtils(41-628)computeFullLeftSourceTableName(625-627)
spark/src/test/scala/ai/chronon/spark/test/join/JoinTest.scala (1)
api/src/main/scala/ai/chronon/api/planner/RelevantLeftForJoinPart.scala (2)
RelevantLeftForJoinPart(15-23)RelevantLeftForJoinPart(25-91)
api/src/main/scala/ai/chronon/api/planner/DependencyResolver.scala (1)
api/src/main/scala/ai/chronon/api/Extensions.scala (3)
Extensions(43-1252)SourceOps(319-432)WindowUtils(88-116)
online/src/main/scala/ai/chronon/online/DataStreamBuilder.scala (3)
api/src/main/scala/ai/chronon/api/DataType.scala (1)
apply(178-178)api/src/main/scala/ai/chronon/api/Builders.scala (9)
apply(30-32)apply(42-65)apply(69-84)apply(88-103)apply(143-167)apply(171-198)apply(202-208)apply(212-217)apply(221-232)api/src/main/scala/ai/chronon/api/ParametricMacro.scala (1)
api(66-66)
spark/src/main/scala/ai/chronon/spark/streaming/GroupBy.scala (3)
api/src/main/scala/ai/chronon/api/Extensions.scala (4)
dataModel(320-325)dataModel(456-463)keys(535-545)keys(808-821)spark/src/main/scala/ai/chronon/spark/stats/drift/Summarizer.scala (1)
keys(103-114)online/src/main/scala/ai/chronon/online/GroupByServingInfoParsed.scala (1)
mutationValueChrononSchema(122-124)
online/src/main/scala/ai/chronon/online/fetcher/GroupByResponseHandler.scala (2)
api/src/main/scala/ai/chronon/api/Extensions.scala (2)
dataModel(320-325)dataModel(456-463)online/src/main/scala/ai/chronon/online/GroupByServingInfoParsed.scala (2)
valueAvroCodec(83-83)mutationValueAvroCodec(126-126)
api/src/main/scala/ai/chronon/api/Extensions.scala (1)
api/src/main/scala/ai/chronon/api/Constants.scala (1)
Constants(23-100)
spark/src/main/scala/ai/chronon/spark/stats/CompareJob.scala (1)
api/src/main/scala/ai/chronon/api/Extensions.scala (2)
dataModel(320-325)dataModel(456-463)
spark/src/main/scala/ai/chronon/spark/GroupByUpload.scala (1)
spark/src/main/scala/ai/chronon/spark/GroupBy.scala (3)
snapshotEvents(183-193)snapshotEntities(153-160)temporalEvents(291-369)
spark/src/main/scala/ai/chronon/spark/GroupBy.scala (2)
api/src/main/scala/ai/chronon/api/Extensions.scala (2)
dataModel(320-325)dataModel(456-463)spark/src/main/scala/ai/chronon/spark/GroupByUpload.scala (2)
snapshotEntities(65-80)snapshotEvents(82-86)
api/src/main/scala/ai/chronon/api/planner/RelevantLeftForJoinPart.scala (1)
api/src/main/scala/ai/chronon/api/Extensions.scala (3)
Extensions(43-1252)SourceOps(319-432)StringOps(656-666)
spark/src/main/scala/ai/chronon/spark/JoinDerivationJob.scala (1)
spark/src/main/scala/ai/chronon/spark/JoinUtils.scala (2)
JoinUtils(41-628)computeFullLeftSourceTableName(625-627)
spark/src/main/scala/ai/chronon/spark/JoinPartJob.scala (4)
spark/src/main/scala/ai/chronon/spark/JoinUtils.scala (3)
JoinUtils(41-628)genBloomFilterIfNeeded(337-374)shiftDays(569-594)api/src/main/scala/ai/chronon/api/Extensions.scala (3)
dataModel(320-325)dataModel(456-463)toPartitionRange(1246-1250)spark/src/main/scala/ai/chronon/spark/GroupBy.scala (2)
snapshotEvents(183-193)snapshotEntities(153-160)api/src/main/scala/ai/chronon/api/DataRange.scala (1)
toPartitionRange(30-32)
spark/src/main/scala/ai/chronon/spark/Analyzer.scala (1)
api/src/main/scala/ai/chronon/api/PartitionSpec.scala (2)
minus(50-50)minus(54-57)
spark/src/main/scala/ai/chronon/spark/LabelJoin.scala (3)
api/src/main/scala/ai/chronon/api/Extensions.scala (2)
dataModel(320-325)dataModel(456-463)spark/src/main/scala/ai/chronon/spark/GroupBy.scala (1)
snapshotEntities(153-160)spark/src/main/scala/ai/chronon/spark/GroupByUpload.scala (1)
snapshotEntities(65-80)
spark/src/test/scala/ai/chronon/spark/test/join/ModularJoinTest.scala (2)
spark/src/main/scala/ai/chronon/spark/JoinUtils.scala (2)
JoinUtils(41-628)computeFullLeftSourceTableName(625-627)api/src/main/scala/ai/chronon/api/planner/RelevantLeftForJoinPart.scala (4)
RelevantLeftForJoinPart(15-23)RelevantLeftForJoinPart(25-91)partTableName(35-53)fullPartTableName(55-64)
spark/src/main/scala/ai/chronon/spark/MergeJob.scala (2)
api/src/main/scala/ai/chronon/api/planner/RelevantLeftForJoinPart.scala (2)
RelevantLeftForJoinPart(15-23)RelevantLeftForJoinPart(25-91)api/src/main/scala/ai/chronon/api/Extensions.scala (2)
dataModel(320-325)dataModel(456-463)
spark/src/main/scala/ai/chronon/spark/Join.scala (2)
api/src/main/scala/ai/chronon/api/planner/RelevantLeftForJoinPart.scala (3)
RelevantLeftForJoinPart(15-23)RelevantLeftForJoinPart(25-91)partTableName(35-53)spark/src/main/scala/ai/chronon/spark/JoinUtils.scala (2)
JoinUtils(41-628)computeFullLeftSourceTableName(625-627)
api/src/main/scala/ai/chronon/api/planner/ConfToNode.scala (5)
api/src/main/scala/ai/chronon/api/Extensions.scala (6)
Extensions(43-1252)MetadataOps(118-165)SourceOps(319-432)table(374-374)from(252-293)outputTable(121-121)spark/src/main/scala/ai/chronon/spark/GroupBy.scala (2)
spark(393-397)from(484-578)spark/src/main/scala/ai/chronon/spark/JoinUtils.scala (2)
JoinUtils(41-628)computeLeftSourceTableName(606-619)spark/src/main/scala/ai/chronon/spark/GroupByUpload.scala (2)
GroupByUpload(56-124)GroupByUpload(126-274)api/src/main/scala/ai/chronon/api/planner/RelevantLeftForJoinPart.scala (3)
RelevantLeftForJoinPart(15-23)RelevantLeftForJoinPart(25-91)partTableName(35-53)
🔇 Additional comments (89)
online/src/main/scala/ai/chronon/online/fetcher/GroupByResponseHandler.scala (2)
85-85: Enum constant renaming looks good.Updated to use
DataModel.ENTITIESinstead ofDataModel.Entitiesfor consistency.
135-137: Pattern matching updated correctly.Changed enum values to uppercase constants (
EVENTS,ENTITIES) to follow standard naming conventions.api/src/main/scala/ai/chronon/api/planner/DependencyResolver.scala (2)
1-1: Package ref updated.
New package correctly reflects the module reorganization.
6-6: Import location revised.
convertUnitsis now imported from its updated location.spark/src/main/scala/ai/chronon/spark/Analyzer.scala (4)
21-22: Updated imports for new DataModel naming convention.Correct update to imports using the new DataModel case objects ENTITIES and EVENTS.
26-26: Updated import path.Import path correctly updated.
437-439: Updated pattern matching for DataModel constants.Pattern matching correctly updated to use ENTITIES instead of the deprecated Entities constant.
440-444: Updated EVENTS pattern matching cases.Pattern matching cases properly updated to use the new EVENTS constant naming convention.
spark/src/main/scala/ai/chronon/spark/MergeJob.scala (4)
15-15: Import change to use ENTITIES constant.Updated import statement to use ENTITIES constant instead of Entities enum.
26-26: Import path adjustment.Import path for RelevantLeftForJoinPart properly specified.
91-91: Updated condition to use ENTITIES constant.Condition correctly updated to maintain the same logic while using the new constant name.
108-108: Updated condition to use ENTITIES constant.Condition in joinWithLeft method properly updated to use the new ENTITIES constant.
spark/src/test/scala/ai/chronon/spark/test/join/ModularJoinTest.scala (6)
6-6: Import updated for package restructuring.Import now specifies planner package which aligns with organizational changes.
201-201: Method name now reflects its full functionality.Changed to
computeFullLeftSourceTableNamewhich accurately describes that it includes namespace information.
280-280: Updated to use planner namespace.Correctly qualified RelevantLeftForJoinPart methods with planner prefix to match package restructuring.
Also applies to: 282-282
295-295: Simplified dataModel parameter usage.Removed unnecessary toString() call since setLeftDataModel now accepts dataModel directly.
305-306: Updated to use planner namespace.Correctly qualified RelevantLeftForJoinPart methods with planner prefix to match package restructuring.
314-314: Simplified dataModel parameter usage.Removed unnecessary toString() call since setLeftDataModel now accepts dataModel directly.
spark/src/main/scala/ai/chronon/spark/streaming/JoinSourceRunner.scala (4)
76-79: Updated DataModel enumeration values to uppercaseChanged pattern matching from
Events/EntitiestoEVENTS/ENTITIES, maintaining consistency with the broader codebase changes.
80-83: Consistent DataModel enumeration usageModified case pattern matching to use uppercase constants (
ENTITIES/EVENTS), aligning with similar changes throughout the codebase.
76-79: Updated pattern matching to use new uppercase data model constants.Constants renamed from lowercase to uppercase for consistency.
80-83: Updated pattern matching to use new uppercase data model constants.Constants renamed from lowercase to uppercase for consistency.
online/src/main/scala/ai/chronon/online/GroupByServingInfoParsed.scala (2)
137-140: Updated DataModel enumeration valuesChanged lowercase
Events/Entitiesto uppercaseEVENTS/ENTITIESin pattern matching, maintaining consistency with the broader enum renaming effort.
138-140: Updated pattern matching to use new uppercase data model constants.Constants renamed from lowercase to uppercase for consistency.
spark/src/test/scala/ai/chronon/spark/test/join/JoinTest.scala (2)
33-33: Updated import path for RelevantLeftForJoinPartImport path now correctly reflects the class's new location in the
ai.chronon.api.plannerpackage.
33-33: Updated import to reflect new package structure.
RelevantLeftForJoinPartmoved fromai.chronon.apitoai.chronon.api.planner.flink/src/main/scala/ai/chronon/flink/AvroCodecFn.scala (2)
59-59: Updated to consistent naming convention.Changed from
DataModel.EventstoDataModel.EVENTSto match the standardized uppercase enum values.
75-76: Updated to consistent naming convention.Changed from
DataModel.EventstoDataModel.EVENTSto match the standardized uppercase enum values.spark/src/main/scala/ai/chronon/spark/SourceJob.scala (3)
3-3: Updated import for new enum value.Direct import of the specific enum value
EVENTSinstead of the wholeDataModelclass.
29-33: Updated to consistent naming convention.Changed from
source.dataModel == Eventstosource.dataModel == EVENTS.
56-60: Updated to consistent naming convention.Changed from
source.dataModel == Eventstosource.dataModel == EVENTS.spark/src/main/scala/ai/chronon/spark/GroupByUpload.scala (1)
236-240: Updated to consistent naming convention.All pattern matching cases updated to use uppercase enum values (
EVENTS,ENTITIES).spark/src/test/scala/ai/chronon/spark/test/OnlineUtils.scala (2)
56-62: Updated to consistent naming convention.Changed from
DataModel.EntitiestoDataModel.ENTITIES.
63-66: Updated to consistent naming convention.Changed from
DataModel.EventstoDataModel.EVENTS.spark/src/main/scala/ai/chronon/spark/LabelJoin.scala (5)
21-22: Updated import statements to use new DataModel constantsImport statements correctly updated from
Entities/EventstoENTITIES/EVENTSto align with API changes.
62-63: Assert correctly updated to use EVENTS constantAssertion updated to use the new
EVENTSconstant.
70-71: Updated assertion to use EVENTS constantConsistent update of assertion condition.
86-87: Updated assertion for ENTITIES constantMaintaining consistent usage of new constant names.
245-249: Updated pattern matching casesPattern matching cases correctly modified to use
EVENTSandENTITIESconstants.spark/src/main/scala/ai/chronon/spark/streaming/GroupBy.scala (5)
55-62: Updated pattern matching for dataModelPattern matching correctly updated to use the new
ENTITIESandEVENTSconstants.
72-75: Updated dataModel case matchingCase matching correctly modified to use the new constant names.
147-148: Updated conditional check for ENTITIESConditional check correctly updated to use the new constant name.
153-155: Updated pattern matching in value columns definitionPattern matching correctly updated to use the new constant names.
164-166: Updated valueZSchema pattern matchingPattern matching for valueZSchema correctly updated to use new constants.
spark/src/main/scala/ai/chronon/spark/stats/CompareJob.scala (2)
21-22: Updated import statementImport statement correctly updated to use new
EVENTSconstant.
177-178: Updated conditional check in getJoinKeysConditional check correctly updated to use the new
EVENTSconstant.spark/src/main/scala/ai/chronon/spark/JoinUtils.scala (7)
20-22: Updated import statementsImport statements correctly updated to use the new
EVENTSconstant.
77-81: Updated conditional check for dataModelConditional check correctly updated to use the new
EVENTSconstant.
107-111: Updated conditional in leftDfFromSourceConditional check correctly updated to use new constant name.
575-579: Updated shiftDays conditional checkConditional correctly updated to use the new
EVENTSconstant.
587-593: Updated rightRange calculationConditional check for calculating rightRange correctly updated.
606-619: Refactored computeLeftSourceTableNameModified to return only the table name without namespace.
621-627: Added computeFullLeftSourceTableName methodNew method correctly combines namespace with table name for complete reference.
api/thrift/api.thrift (1)
497-501: Added DataModel enum in ThriftNew enum with uppercase constants aligns with standardized naming convention across codebase.
spark/src/main/scala/ai/chronon/spark/JoinDerivationJob.scala (1)
26-26: Updated method to use full table nameChanged from
computeLeftSourceTableNametocomputeFullLeftSourceTableNameto use qualified table name with namespace.api/src/test/scala/ai/chronon/api/test/RelevantLeftForJoinPartSpec.scala (1)
5-5: Updated import pathReflects RelevantLeftForJoinPart's move to api.planner package.
online/src/test/scala/ai/chronon/online/test/DataStreamBuilderTest.scala (1)
60-60: Updated to use uppercase DataModel constantChanged from
DataModel.EventstoDataModel.EVENTSto match new enum naming.spark/src/main/scala/ai/chronon/spark/JoinBase.scala (2)
22-22: Updated import to use new constant nameImport updated from
EntitiestoENTITIESto align with standardized constant naming.
77-77: Updated condition check to use the new constantUpdated condition to use
ENTITIESconstant instead ofEntitiesfor consistency.spark/src/main/scala/ai/chronon/spark/BootstrapJob.scala (1)
32-32: Enhanced table name construction logicChanged from
computeLeftSourceTableNametocomputeFullLeftSourceTableNameto include namespace in the table name.api/src/main/scala/ai/chronon/api/planner/RelevantLeftForJoinPart.scala (3)
1-1: Moved to appropriate packageRelocated class to
ai.chronon.api.plannerpackage for better organization.
4-6: Updated importsRemoved specific extension imports and added wildcard import for base package.
79-79: Updated to new constant namingUpdated case match to use
EVENTSinstead ofEventsfor consistency.online/src/main/scala/ai/chronon/online/DataStreamBuilder.scala (3)
58-58: Updated default parameter to new constantChanged default value from
DataModel.EventstoDataModel.EVENTS.
73-77: Updated pattern matching to use new constantsUpdated data model constants in pattern matching to align with new naming convention.
88-91: Updated conditional data model checkModified pattern matching to use the new uppercase constants consistently.
api/src/main/scala/ai/chronon/api/Extensions.scala (1)
322-323: LGTM: Data model constant renamingConstants renamed from
Entities/Eventsto uppercase format.spark/src/main/scala/ai/chronon/spark/GroupBy.scala (3)
34-35: LGTM: Imports updated to use constant namesUpdated imports to reference renamed constants.
435-436: LGTM: Updated DataModel referenceUpdated reference to use new constant name.
739-742: LGTM: Updated pattern matching with new constant namesPattern match now uses imported uppercase constants.
spark/src/main/scala/ai/chronon/spark/JoinPartJob.scala (3)
5-6: LGTM: Updated imports for DataModelImports modified to reference new constant names.
62-62: LGTM: Using node.leftDataModel directlyReplaced local variable with direct property access.
219-232: LGTM: Improved pattern matchingUpdated triple-tuple pattern match with direct property access and new constant names.
spark/src/main/scala/ai/chronon/spark/Join.scala (6)
20-20: Updated import to use new DataModel constant name.The import has been correctly updated from
EntitiestoENTITIES, matching the standardized naming convention in the codebase.
23-23: Explicitly importing planner package.Import now specifically references the planner package rather than relying on wildcard import, improving clarity.
206-206: Updated data model comparison to use ENTITIES constant.Condition properly updated to check against the new uppercase constant name.
338-338: Updated RelevantLeftForJoinPart reference to use planner package.Correctly updated to reference the class from its new location in the planner package.
361-361: Changed to use computeFullLeftSourceTableName.Now using method that includes namespace in the table name, ensuring proper table qualification.
378-378: Updated leftDataModel setter.Direct assignment of data model without conversion, matching the updated type in JoinPartNode.
api/thrift/orchestration.thrift (7)
179-183: Repositioned metaData field to first position.Standardized struct field ordering with metaData as first field.
186-188: Repositioned metaData field to first position.Standardized struct field ordering with metaData as first field.
191-193: Repositioned metaData field to first position.Standardized struct field ordering with metaData as first field.
196-198: Repositioned metaData field to first position.Standardized struct field ordering with metaData as first field.
201-206: Repositioned metaData field and updated leftDataModel type.Standardized struct field ordering and changed leftDataModel from string to api.DataModel enum type.
209-211: Repositioned metaData field to first position.Standardized struct field ordering with metaData as first field.
219-219: Added LabelPartNode to NodeUnion.Added LabelPartNode as the 6th member of the NodeUnion, allowing the use of this node type.
api/src/main/scala/ai/chronon/api/planner/ConfToNode.scala (3)
11-22: Data classes look good.
They are concise and well-structured for holding table/asset info.
24-31: Interface clarity is fine.
Confclearly defines required fields for any implementing configuration.
135-139:JoinConfstructure is clear.
Constructor and basic overrides forjsonandmetaDataare straightforward.
| override def offlineNodes: Seq[Node] = { | ||
|
|
||
| def metaDataCopy(): MetaData = join.metaData.deepCopy() | ||
| def joinCopy(): Join = join.deepCopy() | ||
|
|
||
| require(join.left != null, s"Left side of join ${join.metaData.getName} is null.") | ||
|
|
||
| val leftSourceTableName = JoinUtils.computeLeftSourceTableName(join) | ||
| val sourceMetadata = metaDataCopy().setName(leftSourceTableName) | ||
| val sourceNode = SourceWithFilter(new SourceWithFilterNode() | ||
| .setSource(join.left) | ||
| .setMetaData(sourceMetadata)) | ||
|
|
||
| require(join.metaData != null, s"Join metaData is null for join ${join.metaData.getName}.") | ||
|
|
||
| val bootstrapNodeOpt = Option(join.bootstrapParts) | ||
| .filterNot(_.isEmpty) | ||
| .map{ parts => | ||
| val result = new JoinBootstrapNode() | ||
| result.setJoin(join) | ||
|
|
||
| result.setMetaData(metaDataCopy().setName(join.metaData.getName + ".bootstrap")) | ||
| JoinBootstrap(result) | ||
| } | ||
|
|
||
|
|
||
| val joinPartNodeOpt = for( | ||
| parts <- Option(join.joinParts).toSeq; | ||
| part <- parts.toScala | ||
| ) yield { | ||
| val partOutputTable = RelevantLeftForJoinPart.partTableName(join, part) | ||
| val result = new JoinPartNode() | ||
| result.setLeftSourceTable(sourceMetadata.outputTable) | ||
| result.setJoinPart(part) | ||
| val joinNodes = for( | ||
| source <- part.groupBy.sources.toScala if source.isSetJoinSource; | ||
| joinSource = source.getJoinSource | ||
| effectiveSource = | ||
| ) yield { | ||
| val | ||
| JoinConf(joinSource.getJoin).offlineNodes | ||
| } | ||
| result.setLeftDataModel(join.left.dataModel) | ||
| result.setMetaData(metaDataCopy().setName(partOutputTable)) | ||
| JoinPart(result) | ||
| } | ||
|
|
||
| val mergeNode = { | ||
| val result = new JoinMergeNode() | ||
| val joinCopyObj = joinCopy() | ||
| joinCopyObj.unsetDerivations() | ||
| joinCopyObj.unsetLabelParts() // remove join parts to avoid double counting | ||
| result.setJoin(joinCopyObj) | ||
| result.setMetaData(metaDataCopy().setName(join.metaData.getName + ".merge")) | ||
| JoinMerge(result) | ||
| } | ||
|
|
||
| val derivationNode = { | ||
| val result = new JoinDerivationNode() | ||
| result.setJoin(join) | ||
| result.setMetaData(metaDataCopy().setName(join.metaData.getName + ".derivations")) | ||
| JoinDerivation(result) | ||
| } | ||
|
|
||
| val labelPartNodeOpt = Option(join.labelParts).map { labelParts => | ||
|
|
||
| val result = new LabelPartNode() | ||
| result.setJoin(join) | ||
| result.setMetaData(metaDataCopy().setName(join.metaData.getName + ".label_parts")) | ||
| LabelPart(result) | ||
| } | ||
|
|
||
| (bootstrapNodeOpt ++ | ||
| Seq(sourceNode) ++ | ||
| joinPartNodeOpt ++ | ||
| Seq(mergeNode) ++ | ||
| Seq(derivationNode) ++ | ||
| labelPartNodeOpt).toSeq | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete logic & potential syntax error.
The offline node building is comprehensive but lines 178-181 appear broken. Ensure valid code and properly handle derived offline nodes.
- val
- JoinConf(joinSource.getJoin).offlineNodes
+ // Example fix: gather nodes or remove incomplete snippet
+ val subNodes = JoinConf(joinSource.getJoin).offlineNodesCommittable suggestion skipped: line range outside the PR's diff.
e808406 to
bc59828
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
api/src/main/scala/ai/chronon/api/planner/ConfToNode.scala (1)
175-179: Incomplete code snippet
Similar to a previous review comment: lines are commented out and appear broken. Either remove or complete them.
🧹 Nitpick comments (4)
api/src/main/scala/ai/chronon/api/planner/ConfToNode.scala (4)
29-31: ImplementConf.from.
Currently returns???; provide a concrete implementation for loading configs from filePath.
41-129: Fill in the???methods.
All node subclasses return???; finalize or provide stubs to avoid runtime errors.
218-218: ImplementonlineNodeslogic.
It's unimplemented; define how online nodes are built or return an empty sequence if not applicable.
250-251: Add usage or removetrait Dependency.
It is currently empty; clarify its role or remove it if unnecessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (30)
api/src/main/scala/ai/chronon/api/DataModel.scala(0 hunks)api/src/main/scala/ai/chronon/api/Extensions.scala(6 hunks)api/src/main/scala/ai/chronon/api/planner/ConfToNode.scala(1 hunks)api/src/main/scala/ai/chronon/api/planner/DependencyResolver.scala(1 hunks)api/src/main/scala/ai/chronon/api/planner/RelevantLeftForJoinPart.scala(3 hunks)api/src/test/scala/ai/chronon/api/test/DependencyResolverSpec.scala(1 hunks)api/src/test/scala/ai/chronon/api/test/RelevantLeftForJoinPartSpec.scala(1 hunks)api/thrift/api.thrift(1 hunks)api/thrift/orchestration.thrift(2 hunks)flink/src/main/scala/ai/chronon/flink/AvroCodecFn.scala(2 hunks)online/src/main/scala/ai/chronon/online/DataStreamBuilder.scala(3 hunks)online/src/main/scala/ai/chronon/online/GroupByServingInfoParsed.scala(1 hunks)online/src/main/scala/ai/chronon/online/fetcher/GroupByResponseHandler.scala(2 hunks)online/src/test/scala/ai/chronon/online/test/DataStreamBuilderTest.scala(1 hunks)orchestration/src/main/scala/ai/chronon/orchestration/RepoIndex.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/RepoNodes.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/RepoParser.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/RepoTypes.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/VersionUpdate.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/logical/GroupByNodeImpl.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/logical/JoinNodeImpl.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/logical/LogicalNodeImpl.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/logical/ModelNodeImpl.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/logical/StagingQueryNodeImpl.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/logical/TabularDataNodeImpl.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/physical/ComputeNodeRunner.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/physical/GroupByBackfill.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/physical/GroupByPrepareUpload.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/physical/JoinBackfill.scala(0 hunks)orchestration/src/main/scala/ai/chronon/orchestration/physical/LabelJoin.scala(0 hunks)
💤 Files with no reviewable changes (17)
- orchestration/src/main/scala/ai/chronon/orchestration/physical/GroupByPrepareUpload.scala
- orchestration/src/main/scala/ai/chronon/orchestration/RepoNodes.scala
- orchestration/src/main/scala/ai/chronon/orchestration/logical/LogicalNodeImpl.scala
- orchestration/src/main/scala/ai/chronon/orchestration/physical/LabelJoin.scala
- orchestration/src/main/scala/ai/chronon/orchestration/physical/GroupByBackfill.scala
- orchestration/src/main/scala/ai/chronon/orchestration/logical/JoinNodeImpl.scala
- orchestration/src/main/scala/ai/chronon/orchestration/physical/ComputeNodeRunner.scala
- orchestration/src/main/scala/ai/chronon/orchestration/logical/ModelNodeImpl.scala
- orchestration/src/main/scala/ai/chronon/orchestration/logical/StagingQueryNodeImpl.scala
- orchestration/src/main/scala/ai/chronon/orchestration/RepoIndex.scala
- orchestration/src/main/scala/ai/chronon/orchestration/physical/JoinBackfill.scala
- orchestration/src/main/scala/ai/chronon/orchestration/logical/TabularDataNodeImpl.scala
- orchestration/src/main/scala/ai/chronon/orchestration/RepoParser.scala
- api/src/main/scala/ai/chronon/api/DataModel.scala
- orchestration/src/main/scala/ai/chronon/orchestration/logical/GroupByNodeImpl.scala
- orchestration/src/main/scala/ai/chronon/orchestration/VersionUpdate.scala
- orchestration/src/main/scala/ai/chronon/orchestration/RepoTypes.scala
✅ Files skipped from review due to trivial changes (1)
- api/src/test/scala/ai/chronon/api/test/DependencyResolverSpec.scala
🚧 Files skipped from review as they are similar to previous changes (10)
- online/src/main/scala/ai/chronon/online/GroupByServingInfoParsed.scala
- api/thrift/api.thrift
- flink/src/main/scala/ai/chronon/flink/AvroCodecFn.scala
- online/src/main/scala/ai/chronon/online/fetcher/GroupByResponseHandler.scala
- online/src/test/scala/ai/chronon/online/test/DataStreamBuilderTest.scala
- api/src/main/scala/ai/chronon/api/Extensions.scala
- api/src/main/scala/ai/chronon/api/planner/RelevantLeftForJoinPart.scala
- online/src/main/scala/ai/chronon/online/DataStreamBuilder.scala
- api/src/test/scala/ai/chronon/api/test/RelevantLeftForJoinPartSpec.scala
- api/thrift/orchestration.thrift
🧰 Additional context used
🧬 Code Definitions (2)
api/src/main/scala/ai/chronon/api/planner/DependencyResolver.scala (1)
api/src/main/scala/ai/chronon/api/Extensions.scala (4)
Extensions(40-1262)SourceOps(331-444)WindowUtils(85-126)convertUnits(116-123)
api/src/main/scala/ai/chronon/api/planner/ConfToNode.scala (4)
api/src/main/scala/ai/chronon/api/Extensions.scala (8)
Extensions(40-1262)MetadataOps(128-177)SourceOps(331-444)table(386-386)from(264-305)outputTable(131-131)dataModel(332-337)dataModel(468-475)api/src/main/scala/ai/chronon/api/ThriftJsonCodec.scala (2)
ThriftJsonCodec(41-137)toPrettyJsonStr(56-60)api/src/main/scala/ai/chronon/api/planner/RelevantLeftForJoinPart.scala (2)
RelevantLeftForJoinPart(16-24)RelevantLeftForJoinPart(26-92)api/src/main/scala/ai/chronon/api/Builders.scala (1)
joinSource(132-139)
⏰ Context from checks skipped due to timeout of 90000ms (18)
- GitHub Check: streaming_tests
- GitHub Check: spark_tests
- GitHub Check: streaming_tests
- GitHub Check: join_tests
- GitHub Check: spark_tests
- GitHub Check: join_tests
- GitHub Check: groupby_tests
- GitHub Check: batch_tests
- GitHub Check: fetcher_tests
- GitHub Check: fetcher_tests
- GitHub Check: batch_tests
- GitHub Check: non_spark_tests
- GitHub Check: groupby_tests
- GitHub Check: analyzer_tests
- GitHub Check: non_spark_tests
- GitHub Check: analyzer_tests
- GitHub Check: scala_compile_fmt_fix
- GitHub Check: enforce_triggered_workflows
🔇 Additional comments (3)
api/src/main/scala/ai/chronon/api/planner/DependencyResolver.scala (2)
1-1: Package moved to planner namespacePackage structure rearrangement. Matches PR's structural changes.
5-5: Explicit import of convertUnitsMore specific import improves clarity. Only imports what's needed.
api/src/main/scala/ai/chronon/api/planner/ConfToNode.scala (1)
20-27: Refine theConftrait's contract.
These methods are placeholders; ensure documentation or usage examples clarify expected inputs/outputs.
## Summary - #586 did a refactor on the naming of internal tables - we need to update the Driver code to account for this. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved handling of table name parsing by changing the delimiter from a dot to a double underscore when extracting namespace and table name. - Updated the method for computing source output table names to enhance accuracy in table identification. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> Co-authored-by: Thomas Chow <[email protected]>
## Summary removed some dead code - we are going to introduce planner in a later PR - this PR refactors code around to make the next change easy. ## Checklist - [ ] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new configuration framework that enables enhanced pipeline asset management and clearer node definitions. - Added an updated data model enumeration for improved data categorization. - **Refactor** - Standardized data model identifiers across the system (switching from lower-case to uppercase constants) to ensure consistency. - Updated join table naming logic and reorganized internal components for improved clarity and maintainability. - Streamlined the orchestration subsystem by removing deprecated modules, enhancing overall system performance and reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - #586 did a refactor on the naming of internal tables - we need to update the Driver code to account for this. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved handling of table name parsing by changing the delimiter from a dot to a double underscore when extracting namespace and table name. - Updated the method for computing source output table names to enhance accuracy in table identification. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> Co-authored-by: Thomas Chow <[email protected]>
## Summary removed some dead code - we are going to introduce planner in a later PR - this PR refactors code around to make the next change easy. ## Checklist - [ ] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new configuration framework that enables enhanced pipeline asset management and clearer node definitions. - Added an updated data model enumeration for improved data categorization. - **Refactor** - Standardized data model identifiers across the system (switching from lower-case to uppercase constants) to ensure consistency. - Updated join table naming logic and reorganized internal components for improved clarity and maintainability. - Streamlined the orchestration subsystem by removing deprecated modules, enhancing overall system performance and reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - #586 did a refactor on the naming of internal tables - we need to update the Driver code to account for this. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved handling of table name parsing by changing the delimiter from a dot to a double underscore when extracting namespace and table name. - Updated the method for computing source output table names to enhance accuracy in table identification. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> Co-authored-by: Thomas Chow <[email protected]>
## Summary removed some dead code - we are going to introduce planner in a later PR - this PR refactors code around to make the next change easy. ## Checklist - [ ] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new configuration framework that enables enhanced pipeline asset management and clearer node definitions. - Added an updated data model enumeration for improved data categorization. - **Refactor** - Standardized data model identifiers across the system (switching from lower-case to uppercase constants) to ensure consistency. - Updated join table naming logic and reorganized internal components for improved clarity and maintainability. - Streamlined the orchestration subsystem by removing deprecated modules, enhancing overall system performance and reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - #586 did a refactor on the naming of internal tables - we need to update the Driver code to account for this. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved handling of table name parsing by changing the delimiter from a dot to a double underscore when extracting namespace and table name. - Updated the method for computing source output table names to enhance accuracy in table identification. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> Co-authored-by: Thomas Chow <[email protected]>
## Summary removed some dead code - we are going to introduce planner in a later PR - this PR refactors code around to make the next change easy. ## Checklist - [ ] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new configuration framework that enables enhanced pipeline asset management and clearer node definitions. - Added an updated data model enumeration for improved data categorization. - **Refactor** - Standardized data model identifiers across the system (switching from lower-case to uppercase constants) to ensure consistency. - Updated join table naming logic and reorganized internal components for improved clarity and maintainability. - Streamlined the orchestration subsystem by removing deprecated modules, enhancing overall system performance and reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - #586 did a refactor on the naming of internal tables - we need to update the Driver code to account for this. ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved handling of table name parsing by changing the delimiter from a dot to a double underscore when extracting namespace and table name. - Updated the method for computing source output table names to enhance accuracy in table identification. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to track the status of stacks when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> Co-authored-by: Thomas Chow <[email protected]>
## Summary removed some dead code - we are going to introduce planner in a later PR - this PR refactors code around to make the next change easy. ## Cheour clientslist - [ ] Added Unit Tests - [x] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced a new configuration framework that enables enhanced pipeline asset management and clearer node definitions. - Added an updated data model enumeration for improved data categorization. - **Refactor** - Standardized data model identifiers across the system (switching from lower-case to uppercase constants) to ensure consistency. - Updated join table naming logic and reorganized internal components for improved clarity and maintainability. - Streamlined the orchestration subsystem by removing deprecated modules, enhancing overall system performance and reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - #586 did a refactor on the naming of internal tables - we need to update the Driver code to account for this. ## Cheour clientslist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved handling of table name parsing by changing the delimiter from a dot to a double underscore when extracting namespace and table name. - Updated the method for computing source output table names to enhance accuracy in table identification. <!-- end of auto-generated comment: release notes by coderabbit.ai --> <!-- av pr metadata This information is embedded by the av CLI when creating PRs to traour clients the status of staour clientss when using Aviator. Please do not delete or edit this section of the PR. ``` {"parent":"main","parentHead":"","trunk":"main"} ``` --> Co-authored-by: Thomas Chow <[email protected]>
Summary
removed some dead code - we are going to introduce planner in a later PR - this PR refactors code around to make the next change easy.
Checklist
Summary by CodeRabbit
New Features
Refactor