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

Do not abort if a get_tables fails #106

Merged
merged 2 commits into from
Sep 25, 2020
Merged

Do not abort if a get_tables fails #106

merged 2 commits into from
Sep 25, 2020

Conversation

OssiLehtinen
Copy link
Contributor

An issue connected to the Lake Formation service in AWS appeared:

Lake Formation can be used to share content from one AWS account's glue catalog to another account.

The problem arises, when:

  1. A Lake formation share is created on account A
  2. A resource link is created on account B
  3. The share on acc A is removed leaving an 'oprhaned' resource link

Now, when a user tries to establish a noctua/Athena connection, an error occurs, as the user on account B doesn't have permission to get_tables on account B:s data catalog anymore.

This pull request makes the get_tables error tolerant so that if access is denied, an empty list of tables is put forward.

Probably the multiple tryCatche's are unnecessary now, but I hope this at least outlines the idea how one could address the issue.

Br, Ossi

This is to alleviate a situation where an external lake formation share is destroyed but an orphaned resource link is still present in the local catalog.
@DyfanJones
Copy link
Owner

Hi @OssiLehtinen this logic of this makes sense. Can you apply the same logic to dbGetTables?

We probably want to add some documentation that highlights this behaviour, so that the functions are as transparent to the user.

Also do you mind adding this to the news.md section please?

I am getting ready for the next cran release to resolve issue: #104 . I will hold off until this PR comes into the master branch :)

@OssiLehtinen
Copy link
Contributor Author

I made a note of the fix in news.md.

The original pull request actually has a fix for dbGetTables (and dbListTables) already, sorry for not pointing those out.

@DyfanJones
Copy link
Owner

@OssiLehtinen ah don't worry, it was me missing the change to dbGetTables. I will merge this PR now and get the package ready for it's next cran release :)

@DyfanJones DyfanJones merged commit 9e1eb39 into DyfanJones:master Sep 25, 2020
@DyfanJones
Copy link
Owner

noctua_1.8.1 has been submitted to the cran.

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.

None yet

2 participants