redash
redash copied to clipboard
feature/add support Athena non default catalog
What type of PR is this?
- [x] Refactor
- [x] Feature
- [ ] Bug Fix
- [ ] New Query Runner (Data Source)
- [ ] New Alert Destination
- [ ] Other
Description
Athena Federated Query needs non default catalog_name(AwsDataCatalog). This PR supports another athena catalog_name.
1. boto3, botocore version-up
Now, boto3 / botocore freezed very old version. Athena Federated Query GA 12. 2020.
2. get_schema from Athena-api
Current implementation use Glue-api. athena's catalog_name != glue's catalog_name, ( glue's catalog_name = aws accont-id). So, change get_schema from glue to athena's api. There is no performance difference between glue and athena. When compared with about 2000 tables, only a few seconds difference. I also implemented via information_schema.
3. Keep pyathena 1.x
pyathena 2.x supports non default catalog.But we can set catalog name via sql e.g.) select * from [catalog].[database].[table] So, keep pyathena 1.x, I cloud not find the advantage of connecting by specifying the catalog name. (Even in redash 8, we can execute Federated Query by specifying the catalog name in SQL.)
How is this tested?
- [x] Unit tests (pytest, jest)
- [ ] E2E Tests (Cypress)
- [ ] Manually
- [ ] N/A
Related Tickets & Documents
https://github.com/getredash/redash/pull/5731
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
I found out
An error occurred (UnrecognizedClientException) when calling the StartQueryExecution operation: The security token included in the request is invalid
this error occured after a while...Was it fine for you @h-imaoka @susodapop
I haven't seen this error. My guess is it sounds like your security credential expired. Have you tried renewing it?
Sorry, I tried with IAM Role, and feels like something was mingled with my local machine (where I build my docker image). I guess the PR itself is okay. @susodapop
Please let me know if you're not able to get this working on your box. If there's a change that this pull request breaks existing connections for users we want to know about that before we release it widely.
@h-imaoka , thanks for the PR! We've updated a lot of things now that we're Community-driven so - if you're still interested in getting this merged - would you mind rebasing off master to re-run the CI, as well as updating merge conflicts? We should also address Arik's feedback.
We're trying to clean up our PR todo list, so if you're not interested, that's fine - we'll close the PR in about a week if we don't hear back. If you're interested in reopening the PR afterwards, we would also very much welcome that.