refactor(glue-alpha): glue L2 alpha construct #30833
Conversation
…-glue-l2 to mjanardhan/aws-cdk-glue-l2 to resolve non-trivial merge situation
Workflow Triggers
|
Hi, not sure on the progress of this PR, but I also have a PR open that attempts to tackle Glue Workflows. If this PR doesn't get merged, is it possible to review my PR instead? Thank you. |
We're awaiting review from the CDK service team at the moment, so the plan is still to merge it. @TheRealAmazonKendra is the lead for the original RFP. |
packages/@aws-cdk/aws-glue-alpha/lib/jobs/pyspark-streaming-job.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-glue-alpha/lib/jobs/pyspark-flex-etl-job.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-glue-alpha/lib/jobs/scala-spark-streaming-job.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-glue-alpha/lib/jobs/scala-spark-etl-job.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-glue-alpha/lib/jobs/pyspark-flex-etl-job.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-glue-alpha/lib/jobs/scala-spark-etl-job.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-glue-alpha/lib/jobs/scala-spark-streaming-job.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-glue-alpha/lib/jobs/pyspark-streaming-job.ts
Outdated
Show resolved
Hide resolved
packages/@aws-cdk/aws-glue-alpha/lib/jobs/scala-spark-flex-etl-job.ts
Outdated
Show resolved
Hide resolved
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
|
bump? |
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
3 similar comments
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
|
This PR has been in the BUILD FAILING state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
They've given the review for the PR |
aws-cdk-automation
removed
the
pr/needs-community-review
|
Finally fixed the build. You know, in my spare time. :) Thanks for your patience, everyone. |
…parkUI logging buckets
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
@natalie-white-aws thanks for fixing the failed build. There's still some more conflicts that need to be fixed. I'm happy to give it another pass once that's fixed. |
|
Can you confirm where the conflicts are coming from? I presume on the main branch? I can’t resolve them automatically via GitHub because it says they’re too complex. No one should have been changing the alpha construct while we had an open PR.
README.md has been almost completely overwritten with the refactor.
lib/job.ts was also almost completely rewritten and moved to a new lib/jobs directory.
Integ.job.ts is going away in favor of job-type-specific tests, as is job.test.ts.
***@***.***
Natalie White | AWS Principal Solutions Architect | ***@***.******@***.***> | +1.817.909.5933
From: GZ ***@***.***>
Reply-To: aws/aws-cdk ***@***.***>
Date: Wednesday, November 20, 2024 at 3:06 PM
To: aws/aws-cdk ***@***.***>
Cc: "White, Natalie" ***@***.***>, Mention ***@***.***>
Subject: Re: [aws/aws-cdk] refactor(glue-alpha): glue L2 alpha construct (PR #30833)
@natalie-white-aws<https://github.com/natalie-white-aws> thanks for fixing the failed build. There's still some more conflicts that need to be fixed. I'm happy to give it another pass once that's fixed.
—
Reply to this email directly, view it on GitHub<#30833 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AW64AN3ABYQ2TMIZJEVLCOD2BUIVHAVCNFSM6AAAAABKX5JFTCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOBZG4YTCOBQHE>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
2 similar comments
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
|
@natalie-white-aws I looked into it and I'm not sure how the conflicts happened... It's saying your change is conflict with #12506 from 3 years ago. I would recommend you to rebase your change. The changes in this PR are massive making it extremely hard to review. I went through all the comments left by me and Paul previously and I saw that you've managed to address them mostly. I'll give it another pass this week but it's likely that I don't have any major concern and most feedback are minors so we can have those fixed in a follow-up PR. Once you are able to manage to resolve the conflicts, we can coordinate to have this merged in. |
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
3 similar comments
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
|
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state. |
Issue # (if applicable)
RFC 0497-glue-l2-construct
Link: https://github.com/aws/aws-cdk-rfcs/blob/main/text/0497-glue-l2-construct.md
Reason for this change
Refactor existing Glue L2 alpha construct to adhere to CDK best practices, enforce Job types and parameters via interface constructs, and improve the developer experience pushing validations left and disallowing deprecated Glue and language versions.
Description of changes
The existing Job and Job Executable monoliths have been decomposed into Job Type and Language specific classes that implement and extend an abstract Job parent class. Developers will be able to see mandatory and optional parameters that apply just to their selected job type and language, rather than having to reference documentation and examples or find out during synth or deploy time that they've selected the wrong configuration.
BREAKING CHANGE: Developers must refactor their existing Job instantiation method calls to choose the right job type and language, and use the new constants static values to define the associated Job configuration settings. See the RFC and/or new README for examples.
Description of how you validated changes
We've added unit and integration tests for each valid job type and language combination and validated the creation of new Glue Jobs adhere to opinionated best practices such as continuous logging and runtime versions.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license