[Untested] Vz cherry pick oss avro schema #170

varant-zlai · 2025-01-05T21:46:08Z

Summary

Cherry picking avro schema parsing improvements

Checklist

Added Unit Tests
Covered by existing CI
Integration tested
Documentation update

Summary by CodeRabbit

New Features
- Introduced a new SchemaTraverser trait to enhance schema traversal capabilities.
- Added advanced schema handling for Avro and Spark data conversions.
Improvements
- Enhanced row conversion methods with more flexible schema processing.
- Improved support for complex data types and schema representations.
- Updated encoding and conversion methods across multiple components.
Technical Enhancements
- Implemented AvroSchemaTraverser for more robust Avro schema navigation.
- Refined data conversion methods to support more flexible schema handling.

coderabbitai · 2025-01-05T21:46:14Z

Walkthrough

This pull request introduces a new SchemaTraverser trait and enhances schema handling across multiple Chronon components. The changes focus on improving data conversion and schema traversal capabilities, particularly in Avro and Row-related operations. The modifications allow for more flexible and context-aware schema processing, with updates spanning the API, online, and Spark-related modules.

Changes

File	Change Summary
`api/src/main/scala/ai/chronon/api/Row.scala`	Added `SchemaTraverser` trait, updated `to` method signature to support optional schema traversal
`online/src/main/scala/ai/chronon/online/AvroConversions.scala`	Added `AvroSchemaTraverser`, modified `fromChrononRow` to accept top-level schema, updated encoding methods
`online/src/main/scala/ai/chronon/online/Fetcher.scala`	Updated `encodeOnce` method to pass codec schema to `AvroConversions.fromChrononRow`
`online/src/main/scala/ai/chronon/online/SparkConversions.scala`	Modified `toSparkRow` method to support additional type parameter
`spark/src/main/scala/ai/chronon/spark/utils/InMemoryStream.scala`	Updated `getContinuousStreamDF` to pass Avro schema to conversion method

Possibly Related PRs

Driver Summarizer #62: Direct connection to Avro schema handling changes.
chore: refactor some existing methods #156: Enhances data processing, possibly related to schema handling.
feat: TableUtils to be compatible with DataPointer (part 1) #158: Focuses on data loading, aligns with schema traversal improvements.
feat: support providing additional confs as yaml file for Driver.scala #164: Relates to schema management in Spark jobs.
chore: simplify some Extensions #165: Simplifies data handling extensions, aligning with main PR objectives.

Suggested Reviewers

nikhil-zlai
piyush-zlai
david-zlai

Poem

🌟 Schema's dance, a traversal's art,
Rows transform with a clever start,
Avro's whispers, Spark's embrace,
Code evolves with elegant grace,
Data flows, no structure can part! 🚀

Warning

Review ran into problems

🔥 Problems

GitHub Actions: 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.

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

api/src/main/scala/ai/chronon/api/Row.scala (1)

128-174: Method is lengthy.
Consider breaking out sub-logic to smaller methods for clarity.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between 3aa7369 and 569f5c6.

📒 Files selected for processing (5)

api/src/main/scala/ai/chronon/api/Row.scala (3 hunks)
online/src/main/scala/ai/chronon/online/AvroConversions.scala (4 hunks)
online/src/main/scala/ai/chronon/online/Fetcher.scala (1 hunks)
online/src/main/scala/ai/chronon/online/SparkConversions.scala (1 hunks)
spark/src/main/scala/ai/chronon/spark/utils/InMemoryStream.scala (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms (7)

GitHub Check: table_utils_delta_format_spark_tests
GitHub Check: other_spark_tests
GitHub Check: mutation_spark_tests
GitHub Check: fetcher_spark_tests
GitHub Check: scala_compile_fmt_fix
GitHub Check: join_spark_tests
GitHub Check: no_spark_scala_tests

🔇 Additional comments (7)

online/src/main/scala/ai/chronon/online/AvroConversions.scala (2)

Line range hint 116-138: Ensure null-safety for extraneousRecord.
If extraneousRecord is null, calling it may cause a runtime exception.

190-222: Validate union with two types.
Using get(1) assumes exactly two union types, which can fail otherwise.

api/src/main/scala/ai/chronon/api/Row.scala (2)

43-71: Trait addition looks good.
SchemaTraverser is compact and meets the usage needs.

Line range hint 179-233: Type mismatch risk.
No fallback if item count or types deviate from assumptions in list/map.

online/src/main/scala/ai/chronon/online/SparkConversions.scala (1)

140-143: New signature usage is clear.
Integration with Row.to looks correct.

spark/src/main/scala/ai/chronon/spark/utils/InMemoryStream.scala (1)

103-103: Schema param passed correctly.
AvroConversions call now includes avroSchema properly.

online/src/main/scala/ai/chronon/online/Fetcher.scala (1)

304-304: Good usage of codec schema.
Ensures consistent encoding with the new schema param.

* Cleanup and isolate the schema perf changes * add an _

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

api/src/main/scala/ai/chronon/api/Row.scala (2)
43-71: LGTM! Minor typo in documentation.

Fix typo in comment:
-  // Returns the valye type of the current map field type.
+  // Returns the value type of the current map field type.
202-233: Consider DRYing up map handling logic.

The map handling logic is duplicated between util.Map and immutable.Map cases.

Extract common logic:
def processMapEntry(key: Any, value: Any, keyType: DataType, valueType: DataType, schemaTraverser: Option[SchemaTraverser[OutputSchema]]): (Any, Any) = (
  edit(key, keyType, schemaTraverser.map(_.getMapKeyType)),
  edit(value, valueType, schemaTraverser.map(_.getMapValueType))
)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between 569f5c6 and 9910286.

📒 Files selected for processing (5)

api/src/main/scala/ai/chronon/api/Row.scala (3 hunks)
online/src/main/scala/ai/chronon/online/AvroConversions.scala (4 hunks)
online/src/main/scala/ai/chronon/online/Fetcher.scala (1 hunks)
online/src/main/scala/ai/chronon/online/SparkConversions.scala (1 hunks)
spark/src/main/scala/ai/chronon/spark/utils/InMemoryStream.scala (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

online/src/main/scala/ai/chronon/online/Fetcher.scala
spark/src/main/scala/ai/chronon/spark/utils/InMemoryStream.scala
online/src/main/scala/ai/chronon/online/SparkConversions.scala

⏰ Context from checks skipped due to timeout of 90000ms (7)

GitHub Check: other_spark_tests
GitHub Check: table_utils_delta_format_spark_tests
GitHub Check: join_spark_tests
GitHub Check: mutation_spark_tests
GitHub Check: scala_compile_fmt_fix
GitHub Check: fetcher_spark_tests
GitHub Check: no_spark_scala_tests

🔇 Additional comments (5)

api/src/main/scala/ai/chronon/api/Row.scala (2)

128-136: LGTM! Clean signature update.

The changes maintain backward compatibility while adding schema traversal support.

139-144: LGTM! Well-structured helper functions.

Clean implementation with proper null handling.

online/src/main/scala/ai/chronon/online/AvroConversions.scala (3)

Line range hint 117-142: LGTM! Clean integration with SchemaTraverser.

The changes properly integrate schema traversal capabilities.

Line range hint 176-189: LGTM! Consistent schema handling.

Both encode methods properly utilize the schema traversal capabilities.

194-231: Add unit tests for AvroSchemaTraverser.

The PR objectives indicate missing tests. This new class needs coverage.

Would you like me to generate unit tests for this class?

nikhil-zlai

LGTM!

## Summary Cherry picking avro schema parsing improvements ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update - [x] Untested  ## Summary by CodeRabbit - **New Features** - Introduced a new `SchemaTraverser` trait to enhance schema traversal capabilities. - Added advanced schema handling for Avro and Spark data conversions. - **Improvements** - Enhanced row conversion methods with more flexible schema processing. - Improved support for complex data types and schema representations. - Updated encoding and conversion methods across multiple components. - **Technical Enhancements** - Implemented `AvroSchemaTraverser` for more robust Avro schema navigation. - Refined data conversion methods to support more flexible schema handling.  --------- Co-authored-by: Spencer McNamara <[email protected]> Co-authored-by: ezvz <[email protected]> Co-authored-by: Thomas Chow <[email protected]>

## Summary Cherry picking avro schema parsing improvements ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update - [x] Untested  ## Summary by CodeRabbit - **New Features** - Introduced a new `SchemaTraverser` trait to enhance schema traversal capabilities. - Added advanced schema handling for Avro and Spark data conversions. - **Improvements** - Enhanced row conversion methods with more flexible schema processing. - Improved support for complex data types and schema representations. - Updated encoding and conversion methods across multiple components. - **Technical Enhancements** - Implemented `AvroSchemaTraverser` for more robust Avro schema navigation. - Refined data conversion methods to support more flexible schema handling.  --------- Co-authored-by: Spencer McNamara <[email protected]> Co-authored-by: ezvz <[email protected]>

## Summary Cherry piour clientsing avro schema parsing improvements ## Cheour clientslist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update - [x] Untested  ## Summary by CodeRabbit - **New Features** - Introduced a new `SchemaTraverser` trait to enhance schema traversal capabilities. - Added advanced schema handling for Avro and Spark data conversions. - **Improvements** - Enhanced row conversion methods with more flexible schema processing. - Improved support for complex data types and schema representations. - Updated encoding and conversion methods across multiple components. - **Technical Enhancements** - Implemented `AvroSchemaTraverser` for more robust Avro schema navigation. - Refined data conversion methods to support more flexible schema handling.  --------- Co-authored-by: Spencer McNamara <[email protected]> Co-authored-by: ezvz <[email protected]>

coderabbitai bot reviewed Jan 5, 2025

View reviewed changes

smcnamara2-stripe and others added 3 commits January 7, 2025 15:30

[master] Create and traverse avro schemas once per task (#296)

515fa82

* Cleanup and isolate the schema perf changes * add an _

Include avroSchema in fromChrononRow

026fd25

scalafmt

9910286

varant-zlai force-pushed the vz--cherry_pick_oss_avro_schema branch from 1e28794 to 9910286 Compare January 7, 2025 23:30

coderabbitai bot reviewed Jan 7, 2025

View reviewed changes

nikhil-zlai approved these changes Jan 8, 2025

View reviewed changes

varant-zlai merged commit db26196 into main Jan 8, 2025
10 checks passed

varant-zlai deleted the vz--cherry_pick_oss_avro_schema branch January 8, 2025 01:13

This was referenced Jan 27, 2025

feat: Support Avro date conversions #275

Merged

fix: make logging codec thread safe #857 #298

Closed

This was referenced Feb 14, 2025

Update Catalyst util to bump up whole stage codegen limit for Beacon top #380

Merged

chore: moving join codec from fetcher into metadata store #401

Merged

coderabbitai bot mentioned this pull request Feb 22, 2025

refactor: fetcher sub package + kill old stats in fetcher #423

Merged

4 tasks

coderabbitai bot mentioned this pull request Apr 17, 2025

perf: Online + Avro path optimizations #655

Merged

4 tasks

This was referenced Jun 12, 2025

Update Row conversions to handle Structs coming in as Maps @piyush-zlai #858

Merged

GroupBy derivation fixes #868

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Untested] Vz cherry pick oss avro schema #170

[Untested] Vz cherry pick oss avro schema #170

Uh oh!

varant-zlai commented Jan 5, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jan 5, 2025 •

edited

Loading

Review ran into problems

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

nikhil-zlai left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[Untested] Vz cherry pick oss avro schema #170

[Untested] Vz cherry pick oss avro schema #170

Uh oh!

Conversation

varant-zlai commented Jan 5, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jan 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Possibly Related PRs

Suggested Reviewers

Poem

Review ran into problems

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

nikhil-zlai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

varant-zlai commented Jan 5, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jan 5, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)