Repository navigation
Conversation
c95ee1f to
b12de90
Compare
|
Summary of what changed since the last update on the previous PR. Changes suggested by @yaythomas
Processor retry settings
Destination factories
Fields the service is required to send
Durable processor wrappers
Operation parsing
And resolved some conflicts for the rebase. |
| f"{self.tolerated_failure_count}" | ||
| ) | ||
| raise ValidationError(msg) | ||
| if self.tolerated_failure_percentage is not None and not isinstance( |
There was a problem hiding this comment.
Shouldn't this support fractions also? Can the public and serialization types accept float, while rejecting NaN and values outside 0 through 100?
There was a problem hiding this comment.
Updated tolerated_failure_percentage and the failure_percentage factory to take int | float. I included both because existing code does that, CompletionConfig declares tolerated_failure_percentage: int | float | None. A bare float would accept an int argument too, but it would report the attribute as float | None when an int was stored.
The current check already rejects NaN and values outside 0-100, I added tests for those cases.
| return a ``DistributedMapResult``, which carries the per-item outcomes. | ||
| """ | ||
|
|
||
| result_serdes: SerDes | None = None # None = DEFAULT_JSON_SERDES |
There was a problem hiding this comment.
The result type from result_serdes is lost here. A caller supplying a SerDes[Order] gets list[Any] from DistributedMapResult.get_results().
Can DistributedMapResultConfig, DistributedMapResult, and DistributedMapResultItem carry the result type so get_results() returns list[R], like BatchResult[R]?
There was a problem hiding this comment.
DistributedMapResultConfig, DistributedMapResult and DistributedMapResultItem carry the output type now, so a SerDes[Order] gives DistributedMapResult[Order] and get_results() gives list[Order | None] instead of list[Any]. The None is because a succeeded item can have no output.
This change followed ctx.invoke's example. ctx.map does not actually pass its result type through: its type variable is in no parameter, so ctx.map([1], f).get_results() is list[Never].
| @overload | ||
| def distributed_map( | ||
| self, | ||
| source: DistributedMapSource | Sequence[Any], |
There was a problem hiding this comment.
The signature accepts any Sequence, but the runtime check accepts only DistributedMapSource, list, or tuple. Mypy therefore accepts values such as range(3) and str, which this method then rejects.
Can the annotation name the accepted types, or can the implementation honor the Sequence contract?
There was a problem hiding this comment.
Narrowed source to DistributedMapSource | list[Any] | tuple[Any, ...], in both overloads, the implementation and DistributedMapOperationExecutor.
| *, | ||
| config: DistributedMapConfig | None = None, | ||
| ) -> DistributedMapSummary: | ||
| """Start a distributed map run and resolve with its summary. |
There was a problem hiding this comment.
Since this API is prelaunch, shouldn't DurableContext.distributed_map include the preview check here?
There was a problem hiding this comment.
Did you mean there is an existing preview check I could use here? Couldn't find any example in the SDK.
| result["CallbackOptions"] = self.callback_options.to_dict() | ||
| if self.chained_invoke_options: | ||
| result["ChainedInvokeOptions"] = self.chained_invoke_options.to_dict() | ||
| if self.distributed_map_options: |
There was a problem hiding this comment.
Note: pending model updates in botocore for DistributedMapOptions, DistributedMapDetails, OperationType.DISTRIBUTED_MAP, and the related shapes. Nothing to change in this PR.
| ) -> list[str]: | ||
| """Serialize each inline item, enforcing the 1 MB cap on the whole list.""" | ||
| serialized_items = [self._serialize(serdes, item) for item in items] | ||
| total = len(json.dumps(serialized_items, separators=(",", ":")).encode("utf-8")) |
There was a problem hiding this comment.
Should this calculation use ensure_ascii=False? With a custom serializer that returns non-ASCII text, the default json.dumps behavior counts each character as a \uXXXX escape.
For example, 180,000 é characters count as 1,080,004 bytes here, while their UTF-8 JSON representation is 360,004 bytes. Could we verify which representation the limit applies to and match that representation exactly?
There was a problem hiding this comment.
Updated to use ensure_ascii=False.
The limit is 1 MB on the serialized item collection, measured as the UTF-8 bytes of its compact JSON array, with non-ASCII characters unescaped and JSON's own quote and backslash escaping counted. Confirmed, and the check now matches it exactly.
| self.state.create_checkpoint( | ||
| operation_update=start_operation, is_sync=True | ||
| ) | ||
| except CheckpointError as e: |
There was a problem hiding this comment.
ExecutionState.create_checkpoint(is_sync=True) reports checkpoint-worker failures as BackgroundThreadError, with the original CheckpointError as its source. It does not raise CheckpointError directly here.
The current tests pass because their mock raises CheckpointError directly at this call site. Can this remove the unreachable translation, or test the intended behavior through a real ExecutionState checkpoint worker?
There was a problem hiding this comment.
Removed, with _rejected_config_error. ExecutionState.create_checkpoint reports the failure as BackgroundThreadError, which inherits BaseException, so the except CheckpointError never matched. A rejected START now fails the execution like any other rejected checkpoint, as ctx.invoke does. Tested through a real ExecutionState and checkpoint worker.
Add ctx.distributed_map, which runs a processor function over a large item source and resolves with a summary when the run reaches a terminal state. Covers the configuration types, the handler decorators, the result types and the checkpoint translation.
b12de90 to
5cbea0b
Compare
|
Another update, addressing the above comments plus a few changes to match repo conventions. Shared constants
Removed unused code
Docstrings
Tests
|
Issue #, if available
N/A
Description of changes
Adds the distributed map operation (
ctx.distributed_map) to the Python SDK:A map run processes a bounded dataset in parallel. A customer starts a map run from a durable function, naming a source to read items from, a processor function to invoke per batch, and concurrency, retry, and failure settings. The service reads items from the source, groups them into batches, invokes the processor for each batch, retries failures, tracks progress, routes successful results and failed items to destinations, and reports completion.
config.pyThe input types a customer constructs to describe a map run, and the distributed map enums.
DistributedMapConfig: optional settings for distributed map.DistributedMapResultConfig: subclass that additionally collects item results inline, selecting the return type statically.DistributedMapSource: factories naming where map run items come from.inline,s3_json_lines,s3_json_array,s3_csv,s3_objects,s3_flattened_json_lines,s3_flattened_json_array,s3_flattened_csvandreader_function.S3SourceandReaderSourceare the resolved forms a factory returns.DistributedMapProcessor: describes the Lambda that processes items. Thereport_batch_outcomeandreport_item_outcomesfactories pick how the processor reports outcomes back. Theretryoption takes aDistributedMapProcessorRetryConfigholdingmax_retry_attemptsandmax_retry_duration, withDistributedMapProcessorRetryConfig.UNLIMITEDfor an unbounded attempt count.DistributedMapCompletionConfig: defines item failure thresholds for marking the overall map run failed.DistributedMapDestination: thes3_successesands3_failuresfactories for routing item records to S3.SuccessDestinationandFailureDestinationare the resolved forms, andDistributedMapDestinationConfigholds the pair.UNLIMITED.lambda_service, soconfig.pyhas no runtime import from it.context.pyThe customer-facing entry point on the durable execution context.
ctx.distributed_map: the method a customer calls to run a distributed map. Overloaded so passing aDistributedMapResultConfigtypes the return asDistributedMapResultand anything else asDistributedMapSummary.max_concurrencyto be greater than zero.dmap/models.pyThe result types a customer receives back from a map run.
DistributedMapSummary: whatctx.distributed_mapreturns, describes the run's overall outcome.DistributedMapResult: returned when aDistributedMapResultConfigis passed. Contains individual map run item outcomes.DistributedMapResultItem: represents a single item's result or error.DistributedMapRunArn: the parsed run ARN, exposing the run id.dmap/handlers.pyAuthoring decorators for the processor Lambda, so a customer writes a plain function rather than the item or batch protocol.
distributed_map_item_handler,distributed_map_batch_handler,distributed_map_reader, and the durable variantsdurable_distributed_map_item_handleranddurable_distributed_map_batch_handler.durable_execution, so they work bare, with options, or called directly with a function.boto3_clientandpluginsand pass them through, so a durable processor can register instrumentation plugins or supply its own client.operation/dmap.pyThe executor that drives the operation against the durable execution runtime.
throw_if_error()opts into raising.lambda_service.pySerialization for carrying the operation and its results to and from the backend service.
Wiresuffix.Items,Output, and the record body are opaque strings, matching the API model, so any serdes works rather than JSON only.DistributedMapDetailswhenever the key is present. The service sends the object on every distributed map operation and leaves it empty until the run starts, so an empty object round trips as an empty object.state.pyDurable execution state handling, so a run's outcome persists across suspend and resume.
exceptions.pyThe error type a customer catches.
DistributedMapError: raised when a run or an item fails.plugin.pyOperation type registration.
DISTRIBUTED_MAPtoOperationType.__init__.pyThe package's public API surface.
durable_executionanddurable_stepare exported.config.dmap/__init__.pyEmpty package marker for the
dmappackage.Tests
Tests are split by layer so each sits alongside its module.
tests/config_test.py: config and argument validation.tests/lambda_service_test.pyandtests/dmap/models_test.py: serialization round trips and the result types.tests/operation/dmap_test.pyandtests/context_test.py: executor behaviour and thectx.distributed_mapsurface.tests/dmap/handlers_test.py: authoring wrapper tests, checking that they process items, report failures, and reject bad inputs.tests/e2e/dmap_int_test.pyandtests/e2e/dmap_helpers_int_test.py: end to end tests mocking backend responses, covering suspend and resume, result collection, and throwing on failure.Future tasks
Design
See Rules for every change.
Design note (rule 3), for a new feature, a new option or a public API change: Feature planned internally.
Simpler options considered (rule 2), if the change adds logic: N/A
Net change in source lines and in public API members:
+2916lines insrcand+3952intests.New public members:
ctx.distributed_map, 11 names added to the package__all__, 19 types and 16 factory methods inconfig.py, 4 result types indmap/models.py, 5 authoring decorators, and 1 exception type. 24 dataclasses inlambda_service.pycarry the API shapes.Assumptions
Each assumption about the service, Lambda, the runtime or another library, with its evidence, or marked "unverified" (rule 6):
SUCCEEDED,FAILED,STOPPEDorTIMED_OUT. Unverified.DistributedMapDetails, empty until the run starts. Unverified.DistributedMapDetailscarriesSuccessCount,FailureCountandUnprocessedCount, carriesTotalCountonce the run succeeded, carriesSourceExhaustedonceDistributedMapRunArnis present, and reports a run failure asErrorthere rather than as an operation-level error. Unverified./distributed-map-run/, then a run id containing no further/. Unverified.ResultscarriesItemIdand aStatusofSUCCEEDEDorFAILED, with the item's value inOutputas a string the customer's serdes decodes. Unverified.recordslist, each record holding a non-empty stringitemIdand abodythe customer's serdes decodes. Unverified.itemIdthe processor echoes, and accepts the batch-failure and item-outcome response shapes the wrappers return. Unverified.maxItemsPerPage, andstateas the reader last returned it. Unverified.MaxRetryAttemptsof-1meaning unlimited attempts andMaxRetryDurationSecondsa whole number of seconds. Unverified.Decisions not covered by the issue or the design note
One line each, with the reason (rule 7):
ExecutionError, because the other checks in the same parse path raise it.DistributedMapProcessorRetryConfig.UNLIMITEDis the string"unlimited"rather than the wire sentinel, so a caller never writes a magic number.DistributedMapSource, nameds3_json_linesand so on, so the factory name states the source kind in one call.DistributedMapDestinationcarries the factories andDistributedMapDestinationConfigholdson_successandon_failure, because a destination factory returns one half of that pair rather than the pair itself.max_concurrencyis required and checked only as greater than zero. No upper bound is enforced in the SDK, so a service-side limit change needs no SDK release.python -Oremoves asserts and the check would then be absent from the build that most wants it.boto3_clientandpluginsrather than one config object, becausedurable_executionitself takes those two keyword arguments.Items,Outputand the record body are opaque strings, so a customer serdes other than JSON works without an SDK change.Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.