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

Glue table not visible after upgrading to v2.2 with enable auto created pivot role #979

Closed
TejasRGitHub opened this issue Jan 17, 2024 · 2 comments · Fixed by #980
Closed

Glue table not visible after upgrading to v2.2 with enable auto created pivot role #979

TejasRGitHub opened this issue Jan 17, 2024 · 2 comments · Fixed by #980
Labels
effort: low priority: medium status: in-review This issue has been implemented and is currently in review and waiting for next release type: bug Something isn't working
Milestone
v2.3.0

Comments

@TejasRGitHub
Copy link
Contributor

Describe the bug

While updating data.all to v2.2 with enable auto created pivot role enabled , the pivot role is not formed ( this is fixed in #948 )

Due to this failure, at the time of updating the dataset stacks , zero (0) glue tables fetched by the tables_syncer.py. This causes the LastGlueTableStatus set to Deleted.

Once the pivotRole was corrected and the ECS task for tables syncing is running ( i.e. the tables_syncer.py ), the glue tables are fetched but the LastGlueTableStatus is not updated to InSync .

Due to this, even though the glue tables exists for the dataset on the AWS account, they are not displayed on the UI.

How to Reproduce

  1. Create a dataset with glue tables
  2. Query those tables in DatasetTables table and update the LastGlueTableStatus to Deleted
  3. Check the UI if those tables are not present.
  4. Sync the tables, and check the logs, the tables would be fetched from glue but the LastGlueTableStatus is not updated to InSync

Expected behavior

After tables_syncer.py is running or click on the Synchronize button on the UI, the tables should be properly updated in the RDS with InSync status if they are fetched from the glue boto3 call

Your project

No response

Screenshots

No response

OS

Mac

Python version

3.9

AWS data.all version

2.2

Additional context

No response
@TejasRGitHub TejasRGitHub mentioned this issue Jan 17, 2024
Merged
@dlpzx
Copy link
Contributor

dlpzx commented Jan 18, 2024

I have seen the PR and it is currently being tested. Great catch, thanks for picking it up

@dlpzx dlpzx added type: bug Something isn't working status: in-review This issue has been implemented and is currently in review and waiting for next release priority: medium effort: low labels Jan 18, 2024
noah-paige pushed a commit that referenced this issue Jan 18, 2024
@TejasRGitHub
Bug fix - 948 | Added else if condition to get tables into InSync sta…
ea153d5 
…te (#980)

### Feature or Bugfix
- Bugfix

### Detail

- Added else if condition for getting the glue tables which exist on the
AWS account and also present in RDS dataset tables set the
`LastGlueTableStatus` to `InSync`.

### Testing 

1. Created a dataset with tables
2. Marked the dataset tables `LastGlueTableStatus` to `Deleted`.
3. Checked if the table are now NOT displayed. 
4. Deployed data.all with this fix
5. Now the dataset tables are present on the UI. 

### Relates

- #979

### Security
Please answer the questions below briefly where applicable, or write
`N/A`. Based on
[OWASP 10](https://owasp.org/Top10/en/).

- Does this PR introduce or modify any input fields or queries - this
includes
fetching data from storage outside the application (e.g. a database, an
S3 bucket)? N/A
  - Is the input sanitized?
- What precautions are you taking before deserializing the data you
consume?
  - Is injection prevented by parametrizing queries?
  - Have you ensured no `eval` or similar functions are used?
- Does this PR introduce any functionality or component that requires
authorization? No
- How have you ensured it respects the existing AuthN/AuthZ mechanisms?
  - Are you logging failed auth attempts?
- Are you using or adding any cryptographic features? No
  - Do you use a standard proven implementations?
  - Are the used keys controlled by the customer? Where are they stored?
- Are you introducing any new policies/roles/users? No
  - Have you used the least-privilege principle? How? 


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
@noah-paige noah-paige linked a pull request Jan 18, 2024 that will close this issue
Bug fix - 948 | Added else if condition to get tables into InSync state #980
Merged
@noah-paige noah-paige added this to the v2.3.0 milestone Jan 18, 2024
@noah-paige
Copy link
Contributor

Resolved in PR #980 - closing this issue now

@noah-paige noah-paige removed a link to a pull request Jan 18, 2024
Bug fix - 948 | Added else if condition to get tables into InSync state #980
Merged
@noah-paige noah-paige linked a pull request Jan 18, 2024 that will close this issue
Bug fix - 948 | Added else if condition to get tables into InSync state #980
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
effort: low priority: medium status: in-review This issue has been implemented and is currently in review and waiting for next release type: bug Something isn't working
Projects
None yet
Milestone
v2.3.0
Development

Successfully merging a pull request may close this issue.

Bug fix - 948 | Added else if condition to get tables into InSync state TejasRGitHub/aws-dataall
3 participants
@noah-paige @TejasRGitHub @dlpzx