Step Functions: Support for Aliasing #12326
Merged
Conversation
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
LocalStack Community integration with Pro 2 files ± 0 2 suites ±0 1h 51m 50s ⏱️ - 2m 40s Results for commit 6f52c483. ± Comparison against base commit 288a34a. This pull request skips 3 and un-skips 15 tests.♻️ This comment has been updated with latest results. |
gregfurman
approved these changes
Mar 10, 2025
Comment on lines
1187
to
1192
| for alias in state_machine_revision.aliases: | ||
| if alias.is_router_for(state_machine_version_arn=state_machine_version_arn): | ||
| raise ConflictException( | ||
| "Version to be deleted must not be referenced by an alias. " | ||
| f"Current list of aliases referencing this version: [{alias.name}]" | ||
| ) |
There was a problem hiding this comment.
Should we not be constructing the exception message with multiple aliases?
| ) | ||
| state_machine_revision.delete_version(state_machine_version_arn) | ||
|
|
||
| state_machines.pop(state_machine_version.arn) |
There was a problem hiding this comment.
Suggested change
| state_machines.pop(state_machine_version.arn) | |
| state_machines.pop(state_machine_version.arn, None) |
Probably unlikely but since we're not locking here, that means that if this state machine version has already been deleted, doing a pop from a dictionary could result in aKeyError. Maybe let's be defensive and allow for the None case to be defaulted to.
Comment on lines
1166
to
1170
| if ( | ||
| state_machine_revision := state_machines.get(state_machine_version.source_arn) | ||
| ) is None or not isinstance(state_machine_revision, StateMachineRevision): | ||
| continue | ||
| state_machine_revision.aliases.remove(alias) |
There was a problem hiding this comment.
When theStateMachineRevision is retrieved from the store, this state_machine_revision.aliases.remove(alias) will raise an exception. I noticed this being raised in test_base_lifecycle_create_update_describe.
Perhaps rather use the idempotent discard which wont raise an exception when trying to delete a non-existent key from a set:
Suggested change
| if ( | |
| state_machine_revision := state_machines.get(state_machine_version.source_arn) | |
| ) is None or not isinstance(state_machine_revision, StateMachineRevision): | |
| continue | |
| state_machine_revision.aliases.remove(alias) | |
| if ( | |
| state_machine_revision := state_machines.get(state_machine_version.source_arn) | |
| ) is None or not isinstance(state_machine_revision, StateMachineRevision): | |
| continue | |
| state_machine_revision.aliases.discard(alias) |
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.
Motivation
These changes introduce initial support for AWS Step Functions aliasing features.
Future work on aliasing: CloudFormation support; paging in ListStateMachineAliases
Changes
Testing