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(redshift): column compression encodings and comments can now be customised #23597

Merged
merged 26 commits into from
Feb 10, 2023
Merged

feat(redshift): column compression encodings and comments can now be customised #23597

merged 26 commits into from
Feb 10, 2023

Conversation

Rizxcviii
Copy link
Contributor

@Rizxcviii Rizxcviii commented Jan 6, 2023 •
edited
Loading

This feature request includes additions for compression encoding and comments for table columns. This feature request includes both features in one to close #22506

All Submissions:

Adding new Construct Runtime Dependencies:

  • This PR adds new construct runtime 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

@gitpod-io
Copy link

gitpod-io bot commented Jan 6, 2023

@github-actions github-actions bot added beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1 labels Jan 6, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team January 6, 2023 18:01
@Rizxcviii Rizxcviii changed the title feat(redshift): additions for compression encoding and comments feat(redshift): additions for compression encoding for columns and comments for both tables and columns Jan 6, 2023
TheRealAmazonKendra
TheRealAmazonKendra previously requested changes Jan 18, 2023
View reviewed changes
Copy link
Contributor

@TheRealAmazonKendra TheRealAmazonKendra 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 your contribution! There's a lot going on in this PR and I'm inclined to think it needs to get separated into multiple. There's also a lot of duplication so we should see where we can reuse code instead adding it twice.

@Rizxcviii
addition, modification:
73366fd 
addition: test for chained statements
modification: removing duplicate code
@mergify mergify bot dismissed TheRealAmazonKendra’s stale review January 23, 2023 15:47

Pull request has been modified.

@Rizxcviii Rizxcviii changed the title feat(redshift): additions for compression encoding for columns and comments for both tables and columns feat(redshift): additions for compression encoding and comments for table columns Jan 26, 2023
@Rizxcviii Rizxcviii changed the title feat(redshift): additions for compression encoding and comments for table columns feat(redshift): column compression encodings can now be customised, and comments can now be attached Jan 26, 2023
@comcalvi comcalvi self-assigned this Feb 8, 2023
@mergify mergify bot dismissed comcalvi’s stale review February 9, 2023 11:23

Pull request has been modified.

@Rizxcviii Rizxcviii requested review from comcalvi and TheRealAmazonKendra and removed request for TheRealAmazonKendra and comcalvi February 9, 2023 12:27
comcalvi
comcalvi previously requested changes Feb 9, 2023
View reviewed changes
Copy link
Contributor

@comcalvi comcalvi left a comment

Choose a reason for hiding this comment

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

Looks pretty much perfect, except for the odd spacing that occurs now...that's 100% my fault.

packages/@aws-cdk/aws-redshift/test/database-query-provider/table.test.ts Outdated Show resolved Hide resolved
@mergify mergify bot dismissed comcalvi’s stale review February 10, 2023 09:57

Pull request has been modified.

comcalvi
comcalvi approved these changes Feb 10, 2023
View reviewed changes
Copy link
Contributor

@comcalvi comcalvi 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! This is a neat change

@mergify
Copy link
Contributor

mergify bot commented Feb 10, 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
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

@mergify mergify bot merged commit e89b9ef into aws:main Feb 10, 2023
@mergify
Copy link
Contributor

mergify bot commented Feb 10, 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).

@Rizxcviii Rizxcviii deleted the feature/commentting-encoding branch February 10, 2023 18:01
vinayak-kukreja added a commit that referenced this pull request Feb 14, 2023
@vinayak-kukreja
Revert "feat(redshift): column compression encodings and comments can…
d616923 
… now be customised (#23597)"

This reverts commit e89b9ef.
@vinayak-kukreja vinayak-kukreja mentioned this pull request Feb 14, 2023
Merged
mergify bot pushed a commit that referenced this pull request Feb 14, 2023
@vinayak-kukreja
revert: "feat(redshift): column compression encodings and comments ca…
615e1ce 
…n now be customised" (#24165)

Reverts #23597 since it is failing in pipeline currently.
@Rizxcviii Rizxcviii mentioned this pull request Feb 15, 2023
Merged
mergify bot pushed a commit that referenced this pull request Mar 8, 2023
@Rizxcviii
feat(redshift): column compression encodings and comments can now be …
1ca3e00 
…customised (#24177)

In accordance with #24165, I'm opening the same pull request as before. Not sure if my previous PR #23597 will automatically be "re-merged" in, but if not, then you can review this pull request

Will AGAIN close #22506

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
homakk pushed a commit to homakk/aws-cdk that referenced this pull request Mar 28, 2023
@Rizxcviii @homakk
feat(redshift): column compression encodings and comments can now be …
d9600a3 
…customised (aws#24177)

In accordance with aws#24165, I'm opening the same pull request as before. Not sure if my previous PR aws#23597 will automatically be "re-merged" in, but if not, then you can review this pull request

Will AGAIN close aws#22506

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@filletofish filletofish mentioned this pull request Jul 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@comcalvi comcalvi comcalvi approved these changes

@TheRealAmazonKendra TheRealAmazonKendra TheRealAmazonKendra left review comments

Assignees

@comcalvi comcalvi

Labels
beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

@aws-cdk/aws-redshift-alpha: Add encoding and comments for redshift tables column construct
4 participants
@Rizxcviii @aws-cdk-automation @TheRealAmazonKendra @comcalvi