-
Notifications
You must be signed in to change notification settings - Fork 2
DEVC-601: Add app logic for meaningful error when mismatched app type is used #91
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
Changes from all commits
27eed84
c216d32
fa5e7a5
4404c62
21de5cb
7ee1ddb
b7bb304
84a68a4
473a152
837556d
c099de7
c13c6a9
5f6b248
463950c
49837f7
12f174e
c2c25da
c6c541f
306b081
2209c20
a5c970a
f442cd0
21fce8c
b221f8b
2dea522
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| import contextlib | ||
| import functools | ||
| import json | ||
| import os | ||
| from typing import Any, Dict, Optional, Type | ||
|
|
||
| import pydantic | ||
|
|
||
| from corva.models.base import AppType, RawBaseEvent | ||
| from corva.models.scheduled.raw import RawScheduledEvent | ||
| from corva.models.stream.raw import RawStreamEvent | ||
| from corva.models.task import RawTaskEvent | ||
|
|
||
| MANIFESTED_APP_TYPE_TO_RAW_BASE_EVENT = { | ||
| AppType.TASK: RawTaskEvent, | ||
| AppType.STREAM: RawStreamEvent, | ||
| AppType.SCHEDULER: RawScheduledEvent, | ||
| } | ||
|
|
||
|
|
||
| def find_leaf_subclasses(base_class): | ||
| leaf_classes = [] | ||
| for subclass in base_class.__subclasses__(): | ||
| if not subclass.__subclasses__(): | ||
| leaf_classes.append(subclass) | ||
| else: | ||
| leaf_classes.extend(find_leaf_subclasses(subclass)) | ||
| return leaf_classes | ||
|
|
||
|
|
||
| def get_event_type(aws_event: Any) -> Optional[Type[RawBaseEvent]]: | ||
| all_children = find_leaf_subclasses(RawBaseEvent) | ||
| for child_cls in all_children: | ||
| with contextlib.suppress(pydantic.ValidationError): | ||
| child_cls.from_raw_event(aws_event) | ||
| return child_cls | ||
| return None | ||
|
|
||
|
|
||
| def validate_manifested_type( | ||
| manifest_app_type: 'AppType', raw_event_type: Type[RawBaseEvent] | ||
| ) -> None: | ||
| expected_base_event_cls = MANIFESTED_APP_TYPE_TO_RAW_BASE_EVENT[manifest_app_type] | ||
| if expected_base_event_cls != raw_event_type: | ||
| raise RuntimeError( | ||
| f'Application with type "{manifest_app_type.value}" ' | ||
| f'can\'t invoke a "{raw_event_type}" handler' | ||
| ) | ||
|
|
||
|
|
||
| def validate_event_payload(aws_event: Any, raw_event_type: Type[RawBaseEvent]) -> None: | ||
|
|
||
| if event_cls := get_event_type(aws_event): | ||
| if not issubclass(event_cls, raw_event_type): | ||
| raise RuntimeError( | ||
| f'Application with type "{raw_event_type}" ' | ||
| f'was invoked with "{event_cls}" event type' | ||
| ) | ||
|
|
||
|
|
||
| @functools.lru_cache(maxsize=1) | ||
|
Contributor
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. though(non-blocking): Please be advised that this approach can not work between lamba calls.
Contributor
Author
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. But anyway, I think we can keep such logic here to be prepared for provisioned_concurrency in the future after changes on |
||
| def read_manifest() -> Optional[Dict[str, Any]]: | ||
| manifest_json_path = os.path.join(os.getcwd(), "manifest.json") | ||
| if os.path.exists(manifest_json_path): | ||
| with open(manifest_json_path, "r") as manifest_json_file: | ||
| return json.load(manifest_json_file) | ||
| return None | ||
|
|
||
|
|
||
| def get_manifested_type() -> Optional['AppType']: | ||
| if manifest := read_manifest(): | ||
| application_type = manifest.get("application", {}).get("type") | ||
| return AppType(application_type) if application_type else None | ||
| return None | ||
|
|
||
|
|
||
| def validate_app_type_context( | ||
| aws_event: Any, raw_event_type: Type[RawBaseEvent] | ||
| ) -> None: | ||
| if manifested_type := get_manifested_type(): | ||
| validate_manifested_type(manifested_type, raw_event_type) | ||
| else: | ||
| validate_event_payload(aws_event, raw_event_type) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| VERSION = "1.12.0" | ||
| VERSION = "1.12.1" |
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.
note: I like that we do not have mapping dict now. Also, is there a way to use already existing models for event to identify the app type?
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'm not sure that I've got your point, could you please explain a bit wider 🙏
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.
neatpick: Seems like
AppTypeis type of app defined in manifest.json.