fix: fix datetime serilization for lambda invoker #185
Conversation
|
🤖 Emulator PR Created A draft PR has been created with locked dependencies: ➡️ https://github.com/aws/aws-durable-execution-emulator/pull/194 The emulator will build binaries using the exact testing SDK commit locked in uv.lock. |
wangyb-A
force-pushed
the
fix/invoke_datetime_ser
branch
from
7fb33a5 to
7ed18f3
Compare
|
🔄 Emulator PR Updated The emulator PR has been updated with locked dependencies: ➡️ https://github.com/aws/aws-durable-execution-emulator/pull/194 |
wangyb-A
force-pushed
the
fix/invoke_datetime_ser
branch
from
7ed18f3 to
b10120a
Compare
|
🔄 Emulator PR Updated The emulator PR has been updated with locked dependencies: ➡️ https://github.com/aws/aws-durable-execution-emulator/pull/194 |
- Use smithy datetime format for all serializer - Refactor serilization for filesystem and web module - Remove unused import - Remove unused txt file - More unit tests for web/server
wangyb-A
force-pushed
the
fix/invoke_datetime_ser
branch
from
b10120a to
1c95c5e
Compare
|
🔄 Emulator PR Updated The emulator PR has been updated with locked dependencies: ➡️ https://github.com/aws/aws-durable-execution-emulator/pull/194 |
|
|
||
| def test_datetime_object_hook_converts_timestamp_fields(): | ||
| """Test that datetime_object_hook converts timestamp fields to datetime objects.""" | ||
| from datetime import UTC |
| return super().default(obj) | ||
|
|
||
|
|
||
| def datetime_object_hook(obj): |
There was a problem hiding this comment.
used by the decoders in stores/
maybe rename to decode_datetime_hook?
| FunctionName=function_name, | ||
| InvocationType="RequestResponse", # Synchronous invocation | ||
| Payload=json.dumps(input.to_dict(), default=str), | ||
| Payload=json.dumps(input.to_dict(), cls=DateTimeEncoder), |
There was a problem hiding this comment.
couple of lines up there's an inline import
from aws_durable_execution_sdk_python_testing.exceptions import (
ResourceNotFoundException,
InvalidParameterValueException,
)
please move to top while you're here :)
| if isinstance(obj, dict): | ||
| for key, value in obj.items(): | ||
| if isinstance(value, int | float) and key.endswith( | ||
| ("_timestamp", "_time", "Timestamp", "Time") |
There was a problem hiding this comment.
is there a possiblity this is use anywhere where a customer dict is getting serialized? (i.e if a user-space _timestamp key exists)?
on a quick look it seems that invoker, filesystem and sqllite store are where it's used... and in all these cases we control the datastructures (i.e we know the keynames).
could you please double-check all structures that this serializes and whether ("_timestamp", "_time", "Timestamp", "Time") covers it or is not overly greedy catching things it shouldn't?
This is making me wonder whether we mightn't be better off by making the to_dict and from_dict of the serialization classes just serialize/deserialize to/from int. For the classes in our control here in the testing library. For the classes in the SDK, maybe derive
class OperationWithUnixSerializedTime(Operation):
def to_dict(self) -> MutableMapping[str, Any]:
d = super().to_dict(self)
d["SomethingTimestamp"] = int(d["SomethingTimestamp"].timestamp *1000)
3 participants
Issue #, if available:
Description of changes:
Dependencies
If this PR requires testing against a specific branch of the Python Language SDK (e.g., for unreleased changes), uncomment and specify the branch below. Otherwise, leave commented to use the main branch.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.