fix(aws/scheduler): harden ScheduleGroup reconcile + add lifecycle convergence tests#230
Open
sam-goodwin wants to merge 1 commit intomainfrom
Open
fix(aws/scheduler): harden ScheduleGroup reconcile + add lifecycle convergence tests#230sam-goodwin wants to merge 1 commit intomainfrom
sam-goodwin wants to merge 1 commit intomainfrom
Conversation
…nvergence tests - read() returns Unowned(attrs) when the group lacks alchemy internal tags so first-touch adoption requires --adopt / adopt(true). - Drop the bespoke tag-check ConflictException branch on createScheduleGroup. Replaced with bounded ConflictException retry (15 x 2s) so re-creating a name through a residual DELETING window converges instead of failing. - Diff tags against observed cloud tags (not olds.tags) so adoption rewrites ownership tags, and out-of-band tag drift converges. - conflictRetry on tagResource / untagResource / deleteScheduleGroup. - Re-read final State from the cloud after reconcile rather than guessing. Add lifecycle test suite covering: idempotent redeploy, drift convergence after out-of-band tag mutation, recreate after out-of-band deletion, replace on name change, double-destroy idempotency, and adopt(true) takeover of a foreign-tagged group. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
|
Install the packages built from this commit: alchemy bun add alchemy@https://pkg.ing/alchemy/2398c87@alchemy.run/better-auth bun add @alchemy.run/better-auth@https://pkg.ing/@alchemy.run/better-auth/2398c87@alchemy.run/pr-package bun add @alchemy.run/pr-package@https://pkg.ing/@alchemy.run/pr-package/2398c87 |
Contributor
Website Preview DeployedURL: https://alchemyeffectwebsite-worker-pr-230-asjdr4zten6qn7ha.testing-2b2.workers.dev Built from commit This comment updates automatically with each push. |
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.
Following the SQS hardening template from #184 and the sibling Schedule PR #200. Hardens the EventBridge Scheduler
ScheduleGroupresource. UnlikeSchedule, schedule groups DO support tagging — so ownership detection, adoption gating, and tag-drift convergence are all in scope here.Reconciler changes
readnow returnsUnowned(attrs)when the group lacks the alchemy internal tag set, so first-touch adoption requires--adopt/adopt(true)instead of being silently taken over. Subsequent reconciles trust the persisted output.ConflictExceptionownership check oncreateScheduleGroup. The previous code fetched tags, calledhasTags, and threw a plainErrorif they didn't match — duplicating ownership logic that now lives entirely inread+ the engine's--adoptgate.ConflictExceptionretry (15 × 2s) oncreateScheduleGroup,tagResource,untagResource, anddeleteScheduleGroup. EventBridge Scheduler returnsConflictExceptionwhile a group is in the transientDELETING/CREATINGstate, so re-creating a name through a residualDELETINGwindow now converges instead of failing.Statefrom the cloud after reconcile rather than echoingobserved?.State(which may be stale by one round-trip).ConflictExceptionis intentionally NOT taggedRetryableErrorin distilled — it's context-dependent (DELETINGrace vs. genuine name collision). The reconciler-level bounded retry stays scoped.New lifecycle tests
packages/alchemy/test/AWS/Scheduler/ScheduleGroup.test.ts— each test runsdestroy → deploy → … → destroyon aScratchStackand asserts convergence at every step:DELETINGwindow)nametriggers replace; old group is deletedadopt(true)to take over and rewrites internal alchemy tagsDistilled patch
No patch needed —
packages/aws/patches/scheduler.json(per the existing process) already coversConflictException, and tagging itRetryableErrorwould over-broaden coverage. The reconciler-level bounded retry handles the genuinely transient cases.