-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat: [FC-0074] add support for annotated python dicts as avro map type #433
Open
mariajgrimaldi
wants to merge
13
commits into
main
Choose a base branch
from
MJG/add-support-for-dicts
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
7825196
feat: add support for Avro dict
Ian2012 9aaa04e
refactor: add support for dicts as avro maps instead of record
mariajgrimaldi 375a6ab
chore: generate avro schema for each forum event
mariajgrimaldi 1a5c8a8
refactor: rewrite forum payload to be sent to the event bus
mariajgrimaldi 7f20658
refactor: regenerate avro schemas for latest forum payload
mariajgrimaldi c503783
test: add more test cases for serializing dicts
mariajgrimaldi acdcaa9
refactor: address quality issues from latest commits
mariajgrimaldi 00fd4ac
refactor: address PR reviews
mariajgrimaldi 59e9aae
fix: address quality issues
mariajgrimaldi 172221a
test: add deserializer tests for dicts
mariajgrimaldi 398d4ea
test: add tests for deserialization errors
mariajgrimaldi cbfe065
test: add test cases for schema generation with dict types
mariajgrimaldi f86464a
fix: address quality issues
mariajgrimaldi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,7 +63,7 @@ | |
field["type"] = field_type | ||
# Case 2: data_type is a simple type that can be converted directly to an Avro type | ||
elif data_type in PYTHON_TYPE_TO_AVRO_MAPPING: | ||
if PYTHON_TYPE_TO_AVRO_MAPPING[data_type] in ["record", "array"]: | ||
if PYTHON_TYPE_TO_AVRO_MAPPING[data_type] in ["map", "array"]: | ||
# pylint: disable-next=broad-exception-raised | ||
raise Exception("Unable to generate Avro schema for dict or array fields without annotation types.") | ||
avro_type = PYTHON_TYPE_TO_AVRO_MAPPING[data_type] | ||
|
@@ -83,6 +83,21 @@ | |
f" {set(SIMPLE_PYTHON_TYPE_TO_AVRO_MAPPING.keys())}" | ||
) | ||
field["type"] = {"type": PYTHON_TYPE_TO_AVRO_MAPPING[data_type_origin], "items": avro_type} | ||
elif data_type_origin == dict: | ||
# returns types of dict contents | ||
# if data_type == Dict[str, int], arg_data_type = (str, int) | ||
arg_data_type = get_args(data_type) | ||
if not arg_data_type: | ||
raise TypeError( | ||
mariajgrimaldi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"Dict without annotation type is not supported. The argument should be a type, for eg., Dict[str, int]" | ||
) | ||
avro_type = SIMPLE_PYTHON_TYPE_TO_AVRO_MAPPING.get(arg_data_type[1]) | ||
if avro_type is None: | ||
raise TypeError( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here |
||
"Only following types are supported for dict arguments:" | ||
f" {set(SIMPLE_PYTHON_TYPE_TO_AVRO_MAPPING.keys())}" | ||
) | ||
field["type"] = {"type": PYTHON_TYPE_TO_AVRO_MAPPING[data_type_origin], "values": avro_type} | ||
# Case 3: data_type is an attrs class | ||
elif hasattr(data_type, "__attrs_attrs__"): | ||
# Inner Attrs Class | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
183 changes: 183 additions & 0 deletions
183
...nts/event_bus/avro/tests/schemas/org+openedx+learning+forum+thread+created+v1_schema.avsc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,183 @@ | ||
{ | ||
"name": "CloudEvent", | ||
"type": "record", | ||
"doc": "Avro Event Format for CloudEvents created with openedx_events/schema", | ||
"fields": [ | ||
{ | ||
"name": "thread", | ||
"type": { | ||
"name": "DiscussionThreadData", | ||
"type": "record", | ||
"fields": [ | ||
{ | ||
"name": "body", | ||
"type": "string" | ||
}, | ||
{ | ||
"name": "commentable_id", | ||
"type": "string" | ||
}, | ||
{ | ||
"name": "id", | ||
"type": "string" | ||
}, | ||
{ | ||
"name": "truncated", | ||
"type": "boolean" | ||
}, | ||
{ | ||
"name": "url", | ||
"type": "string" | ||
}, | ||
{ | ||
"name": "user", | ||
"type": { | ||
"name": "UserData", | ||
"type": "record", | ||
"fields": [ | ||
{ | ||
"name": "id", | ||
"type": "long" | ||
}, | ||
{ | ||
"name": "is_active", | ||
"type": "boolean" | ||
}, | ||
{ | ||
"name": "pii", | ||
"type": { | ||
"name": "UserPersonalData", | ||
"type": "record", | ||
"fields": [ | ||
{ | ||
"name": "username", | ||
"type": "string" | ||
}, | ||
{ | ||
"name": "email", | ||
"type": "string" | ||
}, | ||
{ | ||
"name": "name", | ||
"type": "string" | ||
} | ||
] | ||
} | ||
} | ||
] | ||
} | ||
}, | ||
{ | ||
"name": "course_id", | ||
"type": "string" | ||
}, | ||
{ | ||
"name": "thread_type", | ||
"type": [ | ||
"null", | ||
"string" | ||
], | ||
"default": null | ||
}, | ||
{ | ||
"name": "anonymous", | ||
"type": [ | ||
"null", | ||
"boolean" | ||
], | ||
"default": null | ||
}, | ||
{ | ||
"name": "anonymous_to_peers", | ||
"type": [ | ||
"null", | ||
"boolean" | ||
], | ||
"default": null | ||
}, | ||
{ | ||
"name": "title", | ||
"type": [ | ||
"null", | ||
"string" | ||
], | ||
"default": null | ||
}, | ||
{ | ||
"name": "title_truncated", | ||
"type": [ | ||
"null", | ||
"boolean" | ||
], | ||
"default": null | ||
}, | ||
{ | ||
"name": "group_id", | ||
"type": [ | ||
"null", | ||
"long" | ||
], | ||
"default": null | ||
}, | ||
{ | ||
"name": "team_id", | ||
"type": [ | ||
"null", | ||
"long" | ||
], | ||
"default": null | ||
}, | ||
{ | ||
"name": "category_id", | ||
"type": [ | ||
"null", | ||
"long" | ||
], | ||
"default": null | ||
}, | ||
{ | ||
"name": "category_name", | ||
"type": [ | ||
"null", | ||
"string" | ||
], | ||
"default": null | ||
}, | ||
{ | ||
"name": "discussion", | ||
"type": [ | ||
"null", | ||
{ | ||
"type": "map", | ||
"values": "string" | ||
} | ||
], | ||
"default": null | ||
}, | ||
{ | ||
"name": "user_course_roles", | ||
"type": { | ||
"type": "array", | ||
"items": "string" | ||
} | ||
}, | ||
{ | ||
"name": "user_forums_roles", | ||
"type": { | ||
"type": "array", | ||
"items": "string" | ||
} | ||
}, | ||
{ | ||
"name": "options", | ||
"type": { | ||
"type": "map", | ||
"values": "boolean" | ||
} | ||
} | ||
] | ||
} | ||
} | ||
], | ||
"namespace": "org.openedx.learning.forum.thread.created.v1" | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: remove code comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that's commented code but an explanation of what arg_data_type is