search

moj-analytical-services / Rdbtools

Accessing Athena on the Analytical Platform
Other
4 stars 0 forks source link

Get table metadata fix #24

Closed pjrh-moj closed 1 year ago

pjrh-moj commented 1 year ago

After investigating the issue where dbWriteTable broke because dbExistsTable had a new error message from Athena (see here: https://asdslack.slack.com/archives/C8X3PP1TN/p1689335302066809)

Then the fix was applied to noctua: https://github.com/DyfanJones/noctua/issues/205 However, noctua has made an unrelated change which means that some basic things like checking the existence of tables now relies on an API call which requires the athena:GetTableMetadata which standard AP users do not have.

This PR patches the Rbtools version of dbExistsTable so that it catches the new error (it's a bit hacky, but it works fine) and pins the version of noctua to 2.6.1 so that no one ends up with the version which they don't have the right permissions for.

This is probably a short-term fix, as we will now not benefit from future package development. I'm not sure whether this new permission is problematic to be granted, or whether there is another workaround we can implement in Rbtools...

pjrh-moj commented 1 year ago

Thanks Mike - really useful to get confirmation this is a reasonable way of doing it.

Also useful to know this could affect the Python side too. Do you know if the table metadata permission is a problematic one to grant? It would make life a lot easier if we could just have it as part of the standard user permissions. Is it just that it wasn't needed when they set this all up initially, or does it give people more access to information than they should have?

pjrh-moj commented 1 year ago

final minor change which only affects things if an error is thrown and then the retries setting would have remained one.

mratford commented 1 year ago

Thanks Mike - really useful to get confirmation this is a reasonable way of doing it.

Also useful to know this could affect the Python side too. Do you know if the table metadata permission is a problematic one to grant? It would make life a lot easier if we could just have it as part of the standard user permissions. Is it just that it wasn't needed when they set this all up initially, or does it give people more access to information than they should have?

It probably wasn't needed when it was setup, I can't think of any problems that would be caused by granting it. It might be worth trying to get it permitted on the AP so we can let noctua handle any future AWS API changes without constantly having to create workarounds.