Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(stepfunctions-tasks): start glue crawler #29016

Merged
merged 11 commits into from
Mar 11, 2024
Merged

feat(stepfunctions-tasks): start glue crawler #29016

merged 11 commits into from
Mar 11, 2024

Conversation

badmintoncryer
Copy link
Contributor

@badmintoncryer badmintoncryer commented Feb 7, 2024 •
edited
Loading

Issue

Closes #24188.

Reason for this change

AWS Step Functions supports the ability to start a Glue Crawler as a task within a state machine. However, this is not configurable.

Description of changes

I've implemented GlueStartCrawlerRun class in stepfunctions-tasks and we can create start crawler task easily:

  const task = new GlueStartCrawlerRun(stack, 'Task', {
    crawlerName: 'glue-crawler-name',
  });

Description of how you validated changes

I've added both unit and integ tests.

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@github-actions github-actions bot added effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 star-contributor [Pilot] contributed between 25-49 PRs to the CDK labels Feb 7, 2024
@aws-cdk-automation aws-cdk-automation requested a review from a team February 7, 2024 16:45
badmintoncryer
badmintoncryer commented Feb 7, 2024
View reviewed changes
packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/glue/start-crawler-run.ts
/**
* Glue crawler name
*/
readonly crawlerName: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered a specification that receives CfnCfnCrawler instead of crawlerName. The crawler name can be obtained from CfnCrawler.ref.
Which would be preferable?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the current implementation.

@badmintoncryer badmintoncryer force-pushed the 24188-addStartGlueCrawler branch from cea6199 to 8a81e41 Compare February 7, 2024 16:49
@badmintoncryer badmintoncryer marked this pull request as ready for review February 7, 2024 16:49
@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Feb 7, 2024
lpizzinidev
lpizzinidev approved these changes Feb 27, 2024
View reviewed changes
Copy link
Contributor

@lpizzinidev lpizzinidev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍
Just a note on the documentation

packages/aws-cdk-lib/aws-stepfunctions-tasks/README.md Outdated Show resolved Hide resolved
@aws-cdk-automation aws-cdk-automation added pr/needs-maintainer-review This PR needs a review from a Core Team Member and removed pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Feb 27, 2024
@badmintoncryer
Copy link
Contributor Author

@lpizzinidev Thank you for your review!

@github-actions github-actions bot mentioned this pull request Mar 1, 2024
Closed
GavinZZ
GavinZZ reviewed Mar 7, 2024
View reviewed changes
Copy link
Contributor

@GavinZZ GavinZZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, the changes look good just have a nit comments on the documentation.

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/glue/start-crawler-run.ts
/**
* Glue crawler name
*/
readonly crawlerName: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the current implementation.

packages/aws-cdk-lib/aws-stepfunctions-tasks/README.md Outdated
You can call the [`StartCrawler`](https://docs.aws.amazon.com/glue/latest/dg/aws-glue-api-crawler-crawling.html#aws-glue-api-crawler-crawling-StartCrawler) API from a `Task` state through AWS SDK service integrations.

```ts
new tasks.GlueStartCrawlerRun(this, 'Task', {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: might be worth adding another example using the .ref syntax as well as you've mentioned.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added additional example!

@badmintoncryer badmintoncryer force-pushed the 24188-addStartGlueCrawler branch from e12f1fa to 6661ee0 Compare March 8, 2024 16:48
@badmintoncryer
Copy link
Contributor Author

@GavinZZ Thank you for your review!! I've added an example to use CfnCrawler.ref.

GavinZZ
GavinZZ previously approved these changes Mar 11, 2024
View reviewed changes
Copy link
Contributor

@GavinZZ GavinZZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contributing!

@aws-cdk-automation aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Mar 11, 2024
@mergify Mergify
Copy link
Contributor

mergify bot commented Mar 11, 2024

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@badmintoncryer
Copy link
Contributor Author

@GavinZZ Thanks! Mergidy job seems to be failed. Could you please resolve it?

@mergify mergify bot dismissed GavinZZ’s stale review March 11, 2024 23:17

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 19b8555
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Mar 11, 2024
@GavinZZ GavinZZ merged commit 5592553 into aws:main Mar 11, 2024
12 checks passed
@badmintoncryer badmintoncryer deleted the 24188-addStartGlueCrawler branch March 11, 2024 23:53
@aws-cdk-automation aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Mar 11, 2024
This was referenced May 23, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GavinZZ GavinZZ GavinZZ approved these changes

@lpizzinidev lpizzinidev lpizzinidev approved these changes

Assignees
No one assigned
Labels
effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 star-contributor [Pilot] contributed between 25-49 PRs to the CDK
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

(aws_stepfunctions_tasks): (Add support to start glue crawlers)
4 participants
@badmintoncryer @aws-cdk-automation @lpizzinidev @GavinZZ