feat(glue): add ExternalTable for use with connections #24753

Rizxcviii · 2023-03-22T18:19:48Z

Changing the table structure to include an initial TableBase abstract class, allowing different tables of different data sources to be created from. Initially there are two, S3Table and ExternalTable.

S3Table: The current table structure that has been used throughout the previous versions of the CDK
ExternalTable: The new glue table that will be used to store metadata about external data sources. This subclass will contain an externalDataLocation property to explicitly specify the Location property of the underlying CfnTable L1 construct
Table: This is now @deprecated to shift the usage towards S3Table

Closes #24741.

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

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

aws-cdk-automation · 2023-04-01T00:09:46Z

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.

aws-cdk-automation · 2023-04-02T00:10:33Z

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.

aws-cdk-automation · 2023-04-03T00:09:04Z

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.

aws-cdk-automation · 2023-04-13T00:07:44Z

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.

Rizxcviii · 2023-04-13T15:51:28Z

Clarification Request: When performing buildup, it seems to build the entirety of the aws-cdk-lib folder. Is there any way around this, and only build aws-cdk-lib/core or the necessary modules within that folder?

Rizxcviii · 2023-04-14T14:51:16Z

Ignore the request lol, I got it to work

Rizxcviii · 2023-05-04T10:13:22Z

@TheRealAmazonKendra Is it possible to get this PR reviewed please? Thank you.

Rizxcviii · 2023-07-03T13:10:59Z

@corymhall @TheRealAmazonKendra Can this get reviewed? Thank you.

mrgrain

Thanks for this, this will be an important addition to the alpha module.

I'm not quite sure about the design however. The special cases inside the grant*() methods make me think we should have separate subclasses for this. I'm even leaning towards S3Table being the special case of the regular table that just takes a location string.

What do you think?

packages/@aws-cdk/aws-glue-alpha/README.md

packages/@aws-cdk/aws-glue-alpha/lib/table.ts

mrgrain · 2023-09-11T11:02:42Z

@Rizxcviii Very close now! There's still a couple of unexpected changes to the deprecated Table I'd like reverted:

API elements with incompatible changes:
warn - PROP @aws-cdk/aws-glue-alpha.Table.encryption: type Optional<@aws-cdk/aws-glue-alpha.TableEncryption> (formerly @aws-cdk/aws-glue-alpha.TableEncryption): output type is now optional [changed-type:@aws-cdk/aws-glue-alpha.Table.encryption]
warn - INITIALIZER @aws-cdk/aws-glue-alpha.Table.<initializer>: argument props, takes @aws-cdk/aws-glue-alpha.S3TableProps (formerly @aws-cdk/aws-glue-alpha.TableProps): @aws-cdk/aws-glue-alpha.TableProps does not extend @aws-cdk/aws-glue-alpha.S3TableProps [incompatible-argument:@aws-cdk/aws-glue-alpha.Table.<initializer>]
warn - PROP @aws-cdk/aws-glue-alpha.TableProps.bucket: has been removed [removed:@aws-cdk/aws-glue-alpha.TableProps.bucket]
warn - PROP @aws-cdk/aws-glue-alpha.TableProps.encryption: has been removed [removed:@aws-cdk/aws-glue-alpha.TableProps.encryption]
warn - PROP @aws-cdk/aws-glue-alpha.TableProps.encryptionKey: has been removed [removed:@aws-cdk/aws-glue-alpha.TableProps.encryptionKey]
warn - PROP @aws-cdk/aws-glue-alpha.TableProps.s3Prefix: has been removed [removed:@aws-cdk/aws-glue-alpha.TableProps.s3Prefix]

You can check that list yourself by running yarn compat inside the package directory.

packages/@aws-cdk/aws-glue-alpha/test/integ.table.ts

Pull request has been modified.

mrgrain

See other comments.

Pull request has been modified.

Rizxcviii · 2023-09-11T12:01:42Z

@Rizxcviii Very close now! There's still a couple of unexpected changes to the deprecated Table I'd like reverted:

API elements with incompatible changes:
warn - PROP @aws-cdk/aws-glue-alpha.Table.encryption: type Optional<@aws-cdk/aws-glue-alpha.TableEncryption> (formerly @aws-cdk/aws-glue-alpha.TableEncryption): output type is now optional [changed-type:@aws-cdk/aws-glue-alpha.Table.encryption]
warn - INITIALIZER @aws-cdk/aws-glue-alpha.Table.<initializer>: argument props, takes @aws-cdk/aws-glue-alpha.S3TableProps (formerly @aws-cdk/aws-glue-alpha.TableProps): @aws-cdk/aws-glue-alpha.TableProps does not extend @aws-cdk/aws-glue-alpha.S3TableProps [incompatible-argument:@aws-cdk/aws-glue-alpha.Table.<initializer>]
warn - PROP @aws-cdk/aws-glue-alpha.TableProps.bucket: has been removed [removed:@aws-cdk/aws-glue-alpha.TableProps.bucket]
warn - PROP @aws-cdk/aws-glue-alpha.TableProps.encryption: has been removed [removed:@aws-cdk/aws-glue-alpha.TableProps.encryption]
warn - PROP @aws-cdk/aws-glue-alpha.TableProps.encryptionKey: has been removed [removed:@aws-cdk/aws-glue-alpha.TableProps.encryptionKey]
warn - PROP @aws-cdk/aws-glue-alpha.TableProps.s3Prefix: has been removed [removed:@aws-cdk/aws-glue-alpha.TableProps.s3Prefix]

You can check that list yourself by running yarn compat inside the package directory.

This is now fixed. I ran yarn compat after the fix and there appears to be no issues

mergify · 2023-09-11T12:23:51Z

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).

mergify · 2023-09-11T12:27:53Z

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).

Pull request has been modified.

Rizxcviii · 2023-09-11T13:14:17Z

@mrgrain There was an error regarding permissions to update the branch, and it recommended that I update the branch locally, I didn't know that it would remove your approval, my bad :'(

mrgrain · 2023-09-11T13:25:41Z

No worries! Sometimes there are conflicts and then it's unavoidable.

aws-cdk-automation · 2023-09-11T13:35:40Z

AWS CodeBuild CI Report

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

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

mergify · 2023-09-11T13:36:31Z

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).

Changing the table structure to include an initial `TableBase` abstract class, allowing different tables of different data sources to be created from. Initially there are two, `S3Table` and `ExternalTable`. - `S3Table`: The current table structure that has been used throughout the previous versions of the CDK - `ExternalTable`: The new glue table that will be used to store metadata about external data sources. This subclass will contain an `externalDataLocation` property to explicitly specify the `Location` property of the underlying `CfnTable` L1 construct - `Table`: This is now `@deprecated` to shift the usage towards `S3Table` Closes #24741. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Rizxcviii added 3 commits March 22, 2023 12:48

addition: connection name

012df48

addition: custom location of data

cfed556

adding connection integ

13dcaa8

github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 labels Mar 22, 2023

aws-cdk-automation requested a review from a team March 22, 2023 18:20

github-actions bot added the valued-contributor [Pilot] contributed between 6-12 PRs to the CDK label Mar 22, 2023

aws-cdk-automation previously requested changes Mar 22, 2023

View reviewed changes

addition: README

5be4ffc

Merge branch 'main' into feature/glue-connection-for-tables

2172a27

Rizxcviii added 2 commits April 13, 2023 09:56

Merge branch 'main' into feature/glue-connection-for-tables

93923a5

Merge branch 'main' into feature/glue-connection-for-tables

6f337a5

aws-cdk-automation added the pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run label Apr 13, 2023

integ

6581bf4

corymhall removed the pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run label Apr 17, 2023

corymhall assigned TheRealAmazonKendra Apr 20, 2023

corymhall added the @aws-cdk/aws-glue Related to AWS Glue label May 8, 2023

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label May 8, 2023

mrgrain previously requested changes Jul 4, 2023

View reviewed changes

packages/@aws-cdk/aws-glue-alpha/README.md Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-glue-alpha/lib/table.ts Outdated Show resolved Hide resolved

Merge branch 'main' into feature/glue-connection-for-tables

37c4965

Merge branch 'main' into feature/glue-connection-for-tables

693dae0

mrgrain previously requested changes Sep 11, 2023

View reviewed changes

packages/@aws-cdk/aws-glue-alpha/test/integ.table.ts Outdated Show resolved Hide resolved

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Sep 11, 2023

splitting external table into own test file

f0c455b

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Sep 11, 2023

mrgrain previously requested changes Sep 11, 2023

View reviewed changes

compat issues, TableProps is now used for deprecated table

45ea77c

integ test

447acb9

Rizxcviii requested a review from mrgrain September 11, 2023 12:01

mrgrain previously approved these changes Sep 11, 2023

View reviewed changes

mrgrain changed the title ~~feat(glue): glue tables can include glue connections~~ feat(glue): ExternalTable can use connections Sep 11, 2023

mrgrain changed the title ~~feat(glue): ExternalTable can use connections~~ feat(glue): ExternalTable for use with connections Sep 11, 2023

mrgrain changed the title ~~feat(glue): ExternalTable for use with connections~~ feat(glue): add ExternalTable for use with connections Sep 11, 2023

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Sep 11, 2023

Merge branch 'main' into feature/glue-connection-for-tables

e12dee6

mrgrain approved these changes Sep 11, 2023

View reviewed changes

mergify bot merged commit 1c03cb3 into aws:main Sep 11, 2023
10 checks passed

Rizxcviii deleted the feature/glue-connection-for-tables branch September 11, 2023 15:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(glue): add ExternalTable for use with connections #24753

feat(glue): add ExternalTable for use with connections #24753

Rizxcviii commented Mar 22, 2023 •

edited

Loading

aws-cdk-automation left a comment

aws-cdk-automation commented Apr 1, 2023

aws-cdk-automation commented Apr 2, 2023

aws-cdk-automation commented Apr 3, 2023

aws-cdk-automation commented Apr 13, 2023

Rizxcviii commented Apr 13, 2023

Rizxcviii commented Apr 14, 2023

Rizxcviii commented May 4, 2023

Rizxcviii commented Jul 3, 2023

mrgrain left a comment •

edited

Loading

mrgrain commented Sep 11, 2023

mrgrain left a comment •

edited

Loading

Rizxcviii commented Sep 11, 2023

mergify bot commented Sep 11, 2023

mergify bot commented Sep 11, 2023

Rizxcviii commented Sep 11, 2023

mrgrain commented Sep 11, 2023 •

edited

Loading

aws-cdk-automation commented Sep 11, 2023

mergify bot commented Sep 11, 2023

feat(glue): add ExternalTable for use with connections #24753

feat(glue): add ExternalTable for use with connections #24753

Conversation

Rizxcviii commented Mar 22, 2023 • edited Loading

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Apr 1, 2023

aws-cdk-automation commented Apr 2, 2023

aws-cdk-automation commented Apr 3, 2023

aws-cdk-automation commented Apr 13, 2023

Rizxcviii commented Apr 13, 2023

Rizxcviii commented Apr 14, 2023

Rizxcviii commented May 4, 2023

Rizxcviii commented Jul 3, 2023

mrgrain left a comment • edited Loading

Choose a reason for hiding this comment

mrgrain commented Sep 11, 2023

mrgrain left a comment • edited Loading

Choose a reason for hiding this comment

Rizxcviii commented Sep 11, 2023

mergify bot commented Sep 11, 2023

mergify bot commented Sep 11, 2023

Rizxcviii commented Sep 11, 2023

mrgrain commented Sep 11, 2023 • edited Loading

aws-cdk-automation commented Sep 11, 2023

AWS CodeBuild CI Report

mergify bot commented Sep 11, 2023

Rizxcviii commented Mar 22, 2023 •

edited

Loading

mrgrain left a comment •

edited

Loading

mrgrain left a comment •

edited

Loading

mrgrain commented Sep 11, 2023 •

edited

Loading