DUPE (Do not merge) #358
DUPE (Do not merge) #358
Conversation
Fix nested primary key not supported bug
Upstream main
WalkthroughWalkthroughThe changes involve updates to multiple files, including the addition of a new stream configuration for handling primary keys with dots, enhancements to primary key normalization logic, and the introduction of a new test case for normalization behavior. Additionally, the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant System
User->>System: Request to process data stream
System->>System: Normalize primary keys
System->>System: Validate primary keys
alt Validation successful
System->>System: Process data
System->>User: Return processed data
else Validation failed
System->>User: Return error message
end
Wdyt? Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (4)
Files skipped from review due to trivial changes (1)
Additional comments not posted (3)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
tests/integration_tests/fixtures/source-test/source_test/run.py (1)
157-169: The sample record looks great! 🙌It correctly follows the structure defined in the "primary-key-with-dot" stream configuration.
Just one minor suggestion:
sample_record_primary_key_with_dot = { "type": "RECORD", "record": { "stream": "primary-key-with-dot", "emitted_at": 1704067200, "data": { "table1.Column1": "value1", "table1.Column2": 1, "table1.empty_column": None, "table1.big_number": 1234567890123456, }, }, }Wdyt about moving the
"stream"and"emitted_at"keys before the"data"key? This would make it consistent with the other sample records in the file.Other than that, the code changes are approved!
airbyte/shared/catalog_providers.py (1)
157-170: The primary key validation logic looks solid! Just a minor suggestion.The check to ensure that each primary key consists of exactly one node is a good validation step. Raising an
AirbyteErrorwith a detailed message when a key contains more than one node provides clear error reporting.One small suggestion: Maybe we could mention in the error message that the issue is with the stream's configured catalog? Something like:
raise exc.AirbyteError( message=( "Nested primary keys are not supported in the configured catalog. " "Each PK column should have exactly one node. " ), ... )What do you think?
Duplicate of #343. Created in order to see CI test results. Do not merge.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
.gitignorefile to exclude IDE-specific files, maintaining a cleaner repository.