Move DefaultIcebergConfig message to avoid circular dependencies#2104
Move DefaultIcebergConfig message to avoid circular dependencies#2104apilaskowski wants to merge 5 commits into
Conversation
Move DefaultIcebergConfig message from configs.proto to core.proto. This now prevents using core.proto messages in config.proto. The way protos are used across the project lets this change work without additional changes in code.
Removed DefaultIcebergConfig message definition from configs.proto.
apilaskowski
changed the title
| reserved 3, 4, 6, 8, 10, 12, 13; | ||
| } | ||
|
|
||
| message DefaultIcebergConfig { |
There was a problem hiding this comment.
What you're doing is correct (we shouldn't have done this in the first place), but technically it's a breaking change
There was a problem hiding this comment.
@kolina should we leave it as is, or can we introduce this change (I can work around this if we prefer to have it as is).
There was a problem hiding this comment.
Will this change be wire-compatible? i.e. can the new client read the old compilation result protobuf and the old client read the new compilation result?
I would be ok to do this change, but we need to make sure that it won't break runtime interaction
There was a problem hiding this comment.
Apart from looking through the code I did small analysis to confirm those findings.
Generated code from both proto configurations:
Before: DefaultIcebergConfig defined in configs.proto (current main)
After: DefaultIcebergConfig defined in core.proto (PR #2104)
# 1. Generate before_gen/ from main branch schema (DefaultIcebergConfig in configs.proto)
protoc -Ibefore -I. --python_out=before_gen before/configs.proto before/core.proto before/extension.proto
# 2. Generate after_gen/ from PR 2104 schema (DefaultIcebergConfig in core.proto)
protoc -Iafter -I. --python_out=after_gen after/configs.proto after/core.proto after/extension.protoExecuting proto object instantiation and binary serialization produced 100% identical byte outputs.
Also in both cases:
default_iceberg_config on WorkflowSettings resolves to tag 12 with type .dataform.DefaultIcebergConfig.
default_iceberg_config on ProjectConfig resolves to tag 19 with type .dataform.DefaultIcebergConfig.
Removed import statement for core.proto.
2 participants
Move DefaultIcebergConfig message from configs.proto to core.proto.
This now prevents using core.proto messages in config.proto.
The way protos are used across the project lets this change work without additional changes in code.