Repository navigation
fix(sdk): load state with unlisted subtypes - #762
Draft
ParidelPooya wants to merge 1 commit into
Draft
ParidelPooya wants to merge 1 commit into
ParidelPooya wants to merge 1 commit into
Conversation
- OperationSubType(value) returns a member for any non-empty string. A listed value returns its listed member. Any other string returns a member that the enum creates once and reuses, so .value returns the recorded string and `is` still compares equal lookups. - Operation.from_dict and OperationUpdate.from_dict therefore load an operation whose subtype the enum does not list. The SDK parses every operation in the state on each invocation. A subtype recorded by another SDK version, or by a library on top of the SDK, raised ValueError during that parse. The handler then failed before user code ran, even when the code never read that operation. This is a prerequisite for custom subtypes (#749).
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Part of #749. This is the prerequisite that the issue lists as step 2: the SDK must load a subtype that
OperationSubTypedoes not list, before any code records a custom subtype.The defect
Operation.from_dict.Operation.from_dictandOperationUpdate.from_dictcallOperationSubType(data["SubType"]).OperationSubTypeis a closed enum. An unlisted value raisesValueError.Two sources can record such a subtype: another SDK version, and a library built on the SDK. Once #749 adds custom subtypes, a rollback to an SDK version without this fix would fail every execution that recorded one.
The change
OperationSubType._missing_returns a member for any non-empty string:_value2member_map_, so every later lookup returns the same object..valuereturns the recorded string.to_dicttherefore writes the subtype back unchanged.__members__.ValueError.No other source file changes. Parsing, serialization, and the replay identity check work unchanged, because they already use
OperationSubType(...),.value, andis.Why a member, not a plain
str#749 proposed keeping an unlisted subtype as a
str. This PR does not, for one reason:sub_type.value.sub_typeis typedOperationSubType | Nonein the plugin info objects.strhas no.value. So astrwould raiseAttributeErrorin each of these places..value,is, and the declared type correct for every consumer.The follow-up PR for #749 can convert a caller's
sub_type="MySubtype"withOperationSubType("MySubtype"). So the public config can accept astrwhile the rest of the SDK keeps one type.Tests
lambda_service_test.py: the lookup of an unlisted string, rejection of"",None, and3, and round-trips throughOperation.from_dictandOperationUpdate.from_dict.execution_test.py: two tests that pass a raw invocation event.NonDeterministicExecutionError. The message names the checkpoint's subtype.model_test.pyin the testing package:test_events_to_operations_invalid_sub_typeasserted that an unlisted subtype raises. It now asserts that the subtype loads. The testing package source is unchanged. Itstry/except ValueErrorstays, because the testing package accepts core SDK versions from 1.0.0, and those versions still raise.Without the fix, the new core tests fail with
ValueError: 'PyTest...' is not a valid OperationSubType. In the execution tests, the handler itself raises.Verified
isinstance, iteration,__members__,pickle,deepcopy,repr, hashing, and 50 threads that look up the same string at once and all get one object.