feat(glue-alpha): overhaul the workflow trigger API - #38579
Open
otaviomacedo wants to merge 2 commits into
Open
Conversation
Trigger Action and Condition were structs whose mutually-exclusive fields were
policed at synth time. Action had optional job/crawler guarded by an XOR check,
and Condition had optional job/state/crawlerName/crawlState guarded by four
separate throws (job-xor-crawler, plus job-requires-state and
crawler-requires-crawlState). Both made illegal states representable.
Turn each into an abstract class with factory methods:
- Action.job(job, options?) / Action.crawler(crawler, options?)
- Condition.job(job, state, options?) / Condition.crawler(crawler, crawlState, options?)
Selecting job-or-crawler is now a choice of factory, and the state is a required
argument of each factory, so "both", "neither", and "target without state" are
all unrepresentable -- the six runtime throws are removed. Each subtype renders
its own CfnTrigger property via an internal method, so the Workflow no longer
branches on which fields are set.
Actions and conditions now reference their job via IJobRef and their crawler via
ICrawlerRef -- the generated L1 ref interfaces -- rather than the L2 IJob and a
raw L1 CfnCrawler. IJob now extends IJobRef (with a jobRef accessor on JobBase),
so this module's L2 jobs are still accepted directly, while L1 CfnJob/CfnCrawler
and imported refs (CfnJob.fromJobName, CfnCrawler.fromCrawlerName) now
interoperate too. Only the resource name is read here, so the richer L2 surface
was never needed.
Addresses the Action and Condition findings from the aws-glue-alpha pre-GA API
review.
BREAKING CHANGE: trigger `Action` and `Condition` are no longer plain objects.
Use `Action.job(job, { ... })` / `Action.crawler(crawler, { ... })` and
`Condition.job(job, state, { ... })` / `Condition.crawler(crawler, crawlState,
{ ... })`. Jobs are referenced via `IJobRef` and crawlers via `ICrawlerRef` (a
`CfnCrawler` instance or `CfnCrawler.fromCrawlerName(...)`) instead of a
`CfnCrawler` field or crawler-name string. `IJob` now extends the generated
`IJobRef`, so third-party `IJob` implementations must expose a `jobRef` accessor.
Contributor
|
PRs without a linked issue will receive lower priority for review and merging. Please update the description to follow the PR template and include a line like |
otaviomacedo
added
the
pr-linter/exempt-integ-test
aws-cdk-automation
added
the
pr/needs-maintainer-review
`IWorkflow` declared only four of the six trigger-adding methods; the other two
(`addNotifyEventTrigger`, `addConditionalTrigger`) lived on `WorkflowBase` but not
the interface, so workflows obtained via `fromWorkflow*` (typed `IWorkflow`) could
not add event or conditional triggers even though the runtime object supports it.
While fixing this, the trigger method surface — which GA will freeze — is
rationalized in one pass:
- `IWorkflow` now declares every trigger method, so imported workflows have the
full capability set.
- Trigger factory methods return `ITriggerRef` (the generated L1 ref interface)
instead of the raw L1 `CfnTrigger`, matching the "secondary-resource factory
returns the created resource" guidance without leaking an L1 type.
- The three fragmented, hard-coded scheduled methods
(`addDaily/Weekly/CustomScheduledTrigger`) collapse into one
`addScheduledTrigger({ schedule })`, with `TriggerSchedule.daily()` and
`TriggerSchedule.weekly()` convenience factories alongside the existing
`cron()`/`expression()`.
- `addNotifyEventTrigger` is renamed `addEventTrigger` (Glue's type is `EVENT`;
"Notify" was invented terminology); `NotifyEventTriggerOptions` becomes
`EventTriggerOptions`.
- The scheduled/conditional option structs stop chaining
(`ConditionalTriggerOptions extends DailyScheduleTriggerOptions`) and share a
minimal `StartableTriggerOptions` base instead.
Addresses the Workflow & Triggers findings from the aws-glue-alpha pre-GA API
review (interface omission; L1-leaking return type; scheduled over-fragmentation;
event-trigger naming; option-struct inheritance).
BREAKING CHANGE: `addDailyScheduledTrigger`, `addWeeklyScheduledTrigger`, and
`addCustomScheduledTrigger` are replaced by `addScheduledTrigger(id, { schedule,
... })` — use `TriggerSchedule.daily()` / `TriggerSchedule.weekly()` /
`TriggerSchedule.cron(...)` for the schedule. `addNotifyEventTrigger` is renamed
`addEventTrigger` (and `NotifyEventTriggerOptions` to `EventTriggerOptions`). All
`addXxxTrigger` methods now return `ITriggerRef` instead of `CfnTrigger`.
otaviomacedo
changed the title
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
2 participants
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.
Reason for this change
Part of the pre-GA API review of
@aws-cdk/aws-glue-alpha. Several BLOCKING and RECOMMENDED findings in the Workflow & Triggers area, all touching the same trigger surface that GA will freeze:Actionexposed mutually-exclusivejob?/crawler?fields policed by a runtime XOR check.Conditionexposed optionaljob?/state?/crawlerName?/crawlState?policed by four runtime throws.IWorkflowomitted two of the six trigger methods (addNotifyEventTrigger,addConditionalTrigger) — they lived onWorkflowBasebut not the interface, sofromWorkflow*-imported workflows (typedIWorkflow) could not add event or conditional triggers.CfnTrigger(L1 leak).addNotifyEventTriggerused invented terminology (Glue's type isEVENT).Description of changes
Action/Condition→ factory subtypes. Abstract classes with factory methods:Action.job(job, options?)/Action.crawler(crawler, options?)Condition.job(job, state, options?)/Condition.crawler(crawler, crawlState, options?)Choosing job-vs-crawler is now a choice of factory, and the state is a required argument of each condition factory, so "both", "neither", and "target without state" are unrepresentable at compile time. All six
ValidationErrorthrows are removed. Jobs are referenced viaIJobRefand crawlers viaICrawlerRef(IJobnow extendsIJobRefvia ajobRefaccessor onJobBase), so L2 jobs still pass directly while L1CfnJob/CfnCrawlerand imported refs interoperate.Workflow trigger surface rationalized:
IWorkflownow declares every trigger method, so imported workflows have the full capability set.addXxxTriggermethods returnITriggerRef(generated L1 ref interface) instead of rawCfnTrigger— matches the "secondary-resource factory returns the created resource" guidance without leaking an L1 type.addDaily/Weekly/CustomScheduledTriggercollapse into oneaddScheduledTrigger({ schedule }), with newTriggerSchedule.daily()/TriggerSchedule.weekly()factories alongsidecron()/expression().addNotifyEventTrigger→addEventTrigger;NotifyEventTriggerOptions→EventTriggerOptions.StartableTriggerOptionsbase.Description of how you validated changes
Full package unit suite (668 passing) updated to the new API, with added coverage for crawler actions/conditions, an overridden condition logical operator, and an imported
IWorkflowexercising the previously-omitted trigger methods (compile-time proof they are on the interface). Obsolete runtime-throw tests removed (those states no longer compile).integ.workflowverified viainteg-runner --dry-run(no snapshot drift). Package builds clean (jsii compile + lint).BREAKING CHANGE: trigger
ActionandConditionare no longer plain objects — useAction.job(...)/Action.crawler(...)andCondition.job(...)/Condition.crawler(...). Jobs are referenced viaIJobRefand crawlers viaICrawlerRef(aCfnCrawlerinstance orCfnCrawler.fromCrawlerName(...)) instead of aCfnCrawlerfield or crawler-name string;IJobnow extends the generatedIJobRef.addDailyScheduledTrigger/addWeeklyScheduledTrigger/addCustomScheduledTriggerare replaced byaddScheduledTrigger(id, { schedule, ... })(useTriggerSchedule.daily()/weekly()/cron(...)).addNotifyEventTriggeris renamedaddEventTrigger(NotifyEventTriggerOptions→EventTriggerOptions). AlladdXxxTriggermethods now returnITriggerRefinstead ofCfnTrigger.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.