refactor(redshift): UserTablePriviliges to track changes using table IDs'#24874
Conversation
|
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. |
1 similar comment
|
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. |
|
Could I get this reviewed please? |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
This PR has been in the MERGE CONFLICTS 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. |
kaizencc
left a comment
There was a problem hiding this comment.
Sorry for the delayed response here @Rizxcviii. In addition to my comment below about using this.node.id, I don't think this PR is a 'refactor' but rather a 'fix' since we are changing behavior here -- specifically tracking tables based on constructId instead of tableName.
|
This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error. |
|
I'll create a new PR soon and begin working on this again. |
…26955) To support solving #24246, there needs to first be a refactor of the UserTablePriviliges to instead record the table id. The reason being that new privileges would be granted/revoked on old tables that now have new names, since a change in table name caused an UPDATE action to be triggered. However the issue remains where, since the table has a new name, the grant/revoke action will be called on an invalid table name (i.e. the old table name). We now use the table id to track tables, therefore preventing UPDATE events to be triggered. This blocks #24308 This was originally PR #24874, however that had closed. @kaizencc requested changes, that I had added in here. This was originally PR #26558, however that had closed. @kaizencc requested changes, that I have already implemented previously. However, he did not review them. BREAKING CHANGE: the behavior of redshift tables has changed. UPDATE action will not be triggered on new table names and instead be triggered on table id changes. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
To support solving #24246, there needs to first be a refactor of the
UserTablePriviligesto instead record the table id. The reason being that new privileges would be granted/revoked on old tables that now have new names, since a change in table name caused anUPDATEaction to be triggered. However the issue remains where, since the table has a new name, the grant/revoke action will be called on an invalid table name (i.e. the old table name).We now use the table id to track tables, therefore preventing
UPDATEevents to be triggered.This blocks #24308
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license