-
Notifications
You must be signed in to change notification settings - Fork 0
Skip thrift json roundtrip check while reading GroupByServingInfo #774
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 deserialization process for Changes
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (16)
🔇 Additional comments (1)
🪧 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 (
|
## Summary Cherry pick: https://github.com/airbnb/chronon/pull/774/files ## Checklist - [x] 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 - **New Features** - Enhanced group-by request processing with improved key validation - Added a new method for parsing group-by responses with more robust error handling - **Tests** - Added comprehensive test cases for new response parsing method - Verified handling of scenarios with null keys and missing keys <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: ezvz <[email protected]>
## Summary Cherry pick: https://github.com/airbnb/chronon/pull/774/files ## Checklist - [x] 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 - **New Features** - Enhanced group-by request processing with improved key validation - Added a new method for parsing group-by responses with more robust error handling - **Tests** - Added comprehensive test cases for new response parsing method - Verified handling of scenarios with null keys and missing keys <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: ezvz <[email protected]>
## Summary Cherry pick: https://github.com/airbnb/chronon/pull/774/files ## Checklist - [x] 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 - **New Features** - Enhanced group-by request processing with improved key validation - Added a new method for parsing group-by responses with more robust error handling - **Tests** - Added comprehensive test cases for new response parsing method - Verified handling of scenarios with null keys and missing keys <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: ezvz <[email protected]>
## Summary When we add fields in our API, we can run into backwards / forwards compat issues depending on when the json updates make their way out to the GroupByServingInfo (on orch side / serving side). Turning off the round trip check to help cut the noise on these issues. If we can deserialize the thrift json we proceed else this code will throw a JsonException. Some details - [slack thread](https://zipline-2kh4520.slack.com/archives/C08345NBWH4/p1747092844340579) ## 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 - **Bug Fixes** - Improved compatibility when loading certain configuration data by relaxing validation during data processing. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary When we add fields in our API, we can run into backwards / forwards compat issues depending on when the json updates make their way out to the GroupByServingInfo (on orch side / serving side). Turning off the round trip check to help cut the noise on these issues. If we can deserialize the thrift json we proceed else this code will throw a JsonException. Some details - [slack thread](https://zipline-2kh4520.slack.com/archives/C08345NBWH4/p1747092844340579) ## 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 - **Bug Fixes** - Improved compatibility when loading certain configuration data by relaxing validation during data processing. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Cherry piour clients: https://github.com/airbnb/chronon/pull/774/files ## Cheour clientslist - [x] 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 - **New Features** - Enhanced group-by request processing with improved key validation - Added a new method for parsing group-by responses with more robust error handling - **Tests** - Added comprehensive test cases for new response parsing method - Verified handling of scenarios with null keys and missing keys <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: ezvz <[email protected]>
…gInfo (#774) ## Summary When we add fields in our API, we can run into baour clientswards / forwards compat issues depending on when the json updates make their way out to the GroupByServingInfo (on orch side / serving side). Turning off the round trip cheour clients to help cut the noise on these issues. If we can deserialize the thrift json we proceed else this code will throw a JsonException. Some details - [slaour clients thread](https://zipline-2kh4520.slaour clients.com/archives/C08345NBWH4/p1747092844340579) ## 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 - **Bug Fixes** - Improved compatibility when loading certain configuration data by relaxing validation during data processing. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
When we add fields in our API, we can run into backwards / forwards compat issues depending on when the json updates make their way out to the GroupByServingInfo (on orch side / serving side). Turning off the round trip check to help cut the noise on these issues. If we can deserialize the thrift json we proceed else this code will throw a JsonException.
Some details - slack thread
Checklist
Summary by CodeRabbit