fix: ODCS to .proto, problem with array of objects#1012
fix: ODCS to .proto, problem with array of objects#1012jschoedl merged 17 commits intodatacontract:mainfrom
Conversation
schema:
- name: FsaRegister
logicalType: object
properties:
- name: fsa_room
logicalType: array
items:
name: FsaRoom
logicalType: object
properties:
- name: rooms_count
logicalType: integer
- name: category_type
logicalType: string
- name: simple_array_int
logicalType: array
items:
logicalType: integer
- name: simple_int
logicalType: integer
- name: simple_obj
logicalType: object
properties:
- name: rooms_count
logicalType: integer
- name: category_type
logicalType: stringto syntax = "proto3";
package example;
message FsaRegister {
message FsaRoom {
int32 rooms_count = 1;
string category_type = 2;
}
message SimpleObj {
int32 rooms_count = 1;
string category_type = 2;
}
repeated FsaRoom fsa_room = 1;
repeated int32 simple_array_int = 2;
int32 simple_int = 3;
SimpleObj simple_obj = 4;
}
|
|
Sorry, I think I forgot to do the ruff and tests, I'll add them now. |
|
@jochenchrist |
|
I have done ruff check localy, but failed with "uv run pytest", oracle idk... uv run pytest =============================================================== ERRORS ================================================================ |
|
@jochenchrist sooo, lets merge? |
|
@jochenchrist hello! lets merge it? |
There was a problem hiding this comment.
Pull request overview
This PR updates the Protobuf exporter to better handle ODCS schemas—specifically arrays of objects—by generating nested message definitions and updating the expected .proto output accordingly.
Changes:
- Extend the Protobuf export test fixture to include an array-of-objects field (
users) and a non-required field (geo_description). - Refactor Protobuf type/message naming and nested message generation logic to support arrays of objects.
- Add support for emitting
optionalfields forrequired: falseproperties.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
tests/test_export_protobuf.py |
Adds coverage for arrays of objects and required: false → optional output in generated Protobuf. |
datacontract/export/protobuf_exporter.py |
Refactors Protobuf generation to create nested messages (including for arrays of objects) and emit optional for non-required fields. |
Comments suppressed due to low confidence (1)
datacontract/export/protobuf_exporter.py:22
_get_config_valueis typed as returningOptional[str], butcp.valuecan be a non-string (e.g.,dictforenumValues, as used by_get_enum_values). This type mismatch can hide bugs in type checking; consider widening the return type (e.g.,Optional[Any]) or making it generic.
def _get_config_value(prop: SchemaProperty, key: str) -> Optional[str]:
"""Get a custom property value from customProperties."""
if prop.customProperties is None:
return None
for cp in prop.customProperties:
if cp.property == key:
return cp.value
return None
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@jschoedl hi, looks like I fix it, |
…roto-exporter # Conflicts: # datacontract/export/protobuf_exporter.py
…f exporter Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.