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.

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

feat(glue-alpha): add cfn-glue-table-tableinput-parameters to Glue table construct #27643

Merged
merged 6 commits into from
Dec 27, 2023

Conversation

lawofcycles
Copy link
Contributor

@lawofcycles lawofcycles commented Oct 23, 2023

Add cfn-glue-table-tableinput-parameters to Glue Table construct as optional props

User can specify additional table parameter when creating Glue Table.
Any key/value can be set depending on each user's requirement like table's additional metadata or statistics. Some parameter can be used when AWS services / 3rd party tools read table like skip.header.line.count.

Closes #14159.


All Submissions:

  • Have you followed the guidelines in our Contributing guide?
    Adding new Unconventional Dependencies:
  • This PR adds new unconventional dependencies following the process described here
    New Features
  • Have you added the new feature to an integration test?
    • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

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 the beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK label Oct 23, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team October 23, 2023 05:16
@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. p1 labels Oct 23, 2023
@lawofcycles
Copy link
Contributor Author

lawofcycles commented Oct 23, 2023

Note: On current glue-alpha,skip.header.line.count is set on wrong parameter section, cfn-glue-table-storagedescriptor-parameters on table class.
It should be set to cfn-glue-table-tableinput-parameters because other services like Athena expect it. This issue is reported on #27365 and I may fix it on another pull request in the future.

@lawofcycles
Copy link
Contributor Author

lawofcycles commented Oct 23, 2023

It seems integration test failed on CodeBuild. I'll check Build Logs.

Lerna (powered by Nx) Running targets build, test for 63 projects failed

Tasks not run because their dependencies failed or --nx-bail=true:

  • @aws-cdk/aws-apigatewayv2-authorizers-alpha:test
  • @aws-cdk/aws-iot-actions-alpha:test
  • @aws-cdk/aws-iotevents-actions-alpha:test
  • @aws-cdk/aws-scheduler-targets-alpha:test

Failed tasks:

  • @aws-cdk/aws-glue-alpha:test

real 18m14.153s
user 209m8.220s
sys 13m29.698s

@lawofcycles
Copy link
Contributor Author

I have resolved some issues. I would appreciate it if you could review it.

@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Oct 23, 2023
@vinayak-kukreja vinayak-kukreja self-assigned this Nov 17, 2023
@vinayak-kukreja
Copy link
Contributor

Hey @lawofcycles , apologies for the delay and thank you for opening a contribution with us. I will review this PR soon.

@vinayak-kukreja
Copy link
Contributor

Awaiting @TheRealAmazonKendra review on the PR.

@vinayak-kukreja vinayak-kukreja removed their assignment Nov 20, 2023
@bcolas
Copy link

bcolas commented Dec 12, 2023

Any chance to have it reviewed quickly : this is a issue that avoid us to push S3Table in CI/CD to production : no access to change CloudFormation template in production environment

@kaizencc
Copy link
Contributor

FYI: we're in the process of revamping the glue module, which is why I haven't been approving any glue PRs lately. Please bear with us a bit longer, I'm trying to see if we can get the green light to continue to merge quality-of-life PRs in the meantime.

@natalie-white-aws
Copy link

Per Kendra's request, I reviewed these changes and can confirm that these changes do not conflict with the glue-alpha L2 work in flight. Safe to merge pending all other CDK service team checks without impacting our work.

sumupitchayan
sumupitchayan previously approved these changes Dec 27, 2023
Copy link
Contributor

mergify bot commented Dec 27, 2023

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

@aws-cdk-automation aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 27, 2023
Copy link
Contributor

mergify bot commented Dec 27, 2023

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

@sumupitchayan
Copy link
Contributor

@Mergifyio update

Copy link
Contributor

mergify bot commented Dec 27, 2023

update

❌ Mergify doesn't have permission to update

For security reasons, Mergify can't update this pull request. Try updating locally.
GitHub response: refusing to allow a GitHub App to create or update workflow .github/workflows/close-stale-prs.yml without workflows permission

@mergify mergify bot dismissed sumupitchayan’s stale review December 27, 2023 19:34

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: b23bf4f
  • 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 Dec 27, 2023
@sumupitchayan sumupitchayan merged commit 8e15482 into aws:main Dec 27, 2023
12 checks passed
@aws-cdk-automation aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 27, 2023
paulhcsun pushed a commit to paulhcsun/aws-cdk that referenced this pull request Jan 5, 2024
…table construct (aws#27643)

Add
[cfn-glue-table-tableinput-parameters](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-glue-table-tableinput.html#cfn-glue-table-tableinput-parameters)
to Glue Table construct as optional props

User can specify additional table parameter when creating Glue Table. 
Any key/value can be set depending on each user's requirement like
table's additional metadata or statistics. Some parameter can be used
when AWS services / 3rd party tools read table like
`skip.header.line.count`.

Closes aws#14159.

---
All Submissions:
- [x] Have you followed the guidelines in our [Contributing
guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)
Adding new Unconventional Dependencies:
- [ ] This PR adds new unconventional dependencies following the process
described
[here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)
New Features
- [x] Have you added the new feature to an [integration
test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
- [x] Did you use yarn integ to deploy the infrastructure and generate
the snapshot (i.e. yarn integ without --dry-run)?
---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license

---------

Co-authored-by: Vinayak Kukreja <78971045+vinayak-kukreja@users.noreply.github.com>
Co-authored-by: Sumu Pitchayan <35242245+sumupitchayan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(glue): Missing parameter *parameters* from class Table (construct)
7 participants