Adding support for Athena #27
Conversation
sean-garvey
commented
Jul 11, 2017
sean-garvey
commented
- Added Athena Task and tests
- Edited typos in README.md
* Edited typos in README.md
|
Code looks good. Still missing something:
|
* Updated test_validate.py to include test for athena schema * Added athena passing test schema
sean-garvey
changed the title
There was a problem hiding this comment.
Great job @sean-garvey !
I will wait to approve until we are able to test in AWS.
* Updated README.md to reflect change in schema * Updated athena_task.yml to remove unrequired parameters * Updated test_task_runner.py to remove unrequired parameters from the athena task * Updated template.cfn.yml to add S3 permissions necessary for Athena
lambda_cron/schema.json
Outdated
| }, | ||
| "required": ["EncryptionOption"] | ||
| } | ||
| "OutputLocation": { "type": "string" } |
There was a problem hiding this comment.
What about the options "ClientRequestToken" and "QueryExecutionContext": http://boto3.readthedocs.io/en/latest/reference/services/athena.html#Athena.Client.start_query_execution ?
They are not required but if the are used, is the validation ok ? It should allow to use any parameter in the function (start_query_execution) and force required ones.
lambda_cron/template.cfn.yml
Outdated
| Action: | ||
| - athena:StartQueryExecution | ||
| Resource: ['*'] | ||
| - Effect: Allow |
There was a problem hiding this comment.
|
Javier and I have discussed whether or not we should validate AWS API parameters (as the API may evolve over time) and have decided to table to the topic for now. |
* Updated template.cfn.yml to remove extraneous IAM policy statements
|
Tested in AWS, all ok. Feature ready to merge! @sean-garvey ,
|
