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

Introduce sharing State Machines #267

Merged
merged 78 commits into from
Feb 1, 2023
Merged

Conversation

dlpzx
Copy link
Contributor

@dlpzx dlpzx commented Jan 4, 2023

Feature or Bugfix

  • Feature
  • Refactoring

Detail

  • embed sharing actions into State machines
  • Redesign the sharing workflow to keep track of revoke of access
  • Adjust and simplify sharing managers and tasks
  • Redesign clean-up process
  • Added info in share statistics in inbox and outbox:

image

(Check out the conversation in the below issue for more details)

Relates

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

@dlpzx dlpzx requested review from noah-paige and LEUNGUU January 4, 2023 07:59
@dlpzx
Copy link
Contributor Author

dlpzx commented Jan 26, 2023

@noah-paige @degoldner
Fixed issues:

  • failure and cleanup logic

Found issues:

  1. folder sharing failure when folder is shared with a second group
  2. pagination issue in revoke items window for more than 10 items
  3. refractoring tests (optional)

@noah-paige noah-paige self-requested a review January 26, 2023 16:39
@dlpzx
Copy link
Contributor Author

dlpzx commented Jan 30, 2023

After fixing:

  • folder sharing failure when folder is shared with a second group --> it was due to the naming convention needed for access points that did not allow "_"
  • pagination issue in revoke items window for more than 10 items --> filtering defaults

Today I will work on:

  • improving UI for revoke modal so that it refreshes the table of items
  • improving testing of share API and make the tests independent from each other

@dlpzx
Copy link
Contributor Author

dlpzx commented Jan 30, 2023

@degoldner I refractored the sharing api tests. Thank you for pointing it out, I like this last version better :)

Copy link
Contributor

@noah-paige noah-paige left a comment

Choose a reason for hiding this comment

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

I have gone through testing all the new and existing sharing functionality for folders and tables - all seems to be working well and really enjoy the new design and sharing process leveraging the Share SMs

I opened 2 more general issues that I found that are not specific to sharing ( #288 and #287 ) otherwise this PR lgtm

@dlpzx dlpzx merged commit 15aea8c into v1m4m0 Feb 1, 2023
@dlpzx dlpzx mentioned this pull request Feb 2, 2023
dlpzx added a commit that referenced this pull request Feb 3, 2023
- Feature
- Refactoring

- Embed sharing actions into code defined State machines
- Redesign the sharing workflow to keep track of revoke of access
- Adjust and simplify sharing managers and tasks - separation of approval and revoke process
- Revoke items in batch
- Added info in share statistics in inbox and outbox
- Added testing for S3 and LakeFormation share managers and for sharing APIs

(Check out the conversation in the below issue for more details)

- #255

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

---------

Co-authored-by: Dennis Goldner <[email protected]>
dlpzx added a commit that referenced this pull request Feb 7, 2023
### Feature or Bugfix
- Bugfix

### Detail
- Issue on revoking principals in sharing

### Relates
- #267 

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

---------

Co-authored-by: Dennis Goldner <[email protected]>
dlpzx added a commit that referenced this pull request Feb 9, 2023
### Feature or Bugfix
- v.1.4.0 Features

### Detail
- #267 - Sharing State Machines
- #289 - Visibility of shared access points and databases
- #259  - Pipeline filter to search them

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

---------

Co-authored-by: Noah Paige <[email protected]>
Co-authored-by: Dennis Goldner <[email protected]>
noah-paige pushed a commit to noah-paige/aws-dataall that referenced this pull request Mar 2, 2023
- Feature
- Refactoring

- Embed sharing actions into code defined State machines
- Redesign the sharing workflow to keep track of revoke of access
- Adjust and simplify sharing managers and tasks - separation of approval and revoke process
- Revoke items in batch
- Added info in share statistics in inbox and outbox
- Added testing for S3 and LakeFormation share managers and for sharing APIs

(Check out the conversation in the below issue for more details)

- data-dot-all#255

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

---------

Co-authored-by: Dennis Goldner <[email protected]>
dlpzx added a commit that referenced this pull request Mar 6, 2023
### Feature or Bugfix
- v.1.4.0 Features

### Detail
- #267 - Sharing State Machines
- #289 - Visibility of shared access points and databases
- #259  - Pipeline filter to search them

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

---------

Co-authored-by: Noah Paige <[email protected]>
Co-authored-by: Dennis Goldner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants