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

[AIRFLOW-3266] Add AWS Athena Operator and hook #4111

Merged
merged 1 commit into from Nov 16, 2018

Conversation

Projects
None yet
10 participants
@phani8996
Copy link
Contributor

commented Oct 28, 2018

Jira

Description

  • There is no official athena operator as of now airflow. Either one has do it using boto3 in python operator or using aws cli in bash operator. Either of these does not take care of total life cycle of query. Create a Athena operator and hook to submit presto query and update task based of state of submitted query.

Tests

  • I added test to assert query parameters and mocked hook methods for unit testing the oprtaor. Unfortunately library moto which used for unit testing aws services does not support at this point of time.

Commits

  • My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it.
    • When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.

Code Quality

  • Passes flake8

@phani8996 phani8996 force-pushed the phani8996:AIRFLOW-3266 branch Oct 28, 2018

@codecov-io

This comment has been minimized.

Copy link

commented Oct 28, 2018

Codecov Report

Merging #4111 into master will decrease coverage by 1.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4111      +/-   ##
==========================================
- Coverage    77.7%   76.67%   -1.04%     
==========================================
  Files         199      199              
  Lines       16309    16212      -97     
==========================================
- Hits        12673    12430     -243     
- Misses       3636     3782     +146
Impacted Files Coverage Δ
airflow/operators/postgres_operator.py 0% <0%> (-100%) ⬇️
airflow/operators/mysql_to_hive.py 0% <0%> (-100%) ⬇️
airflow/operators/mysql_operator.py 0% <0%> (-100%) ⬇️
airflow/operators/generic_transfer.py 0% <0%> (-100%) ⬇️
airflow/www_rbac/utils.py 68.94% <0%> (-5.15%) ⬇️
airflow/hooks/dbapi_hook.py 79.83% <0%> (-3.23%) ⬇️
airflow/hooks/postgres_hook.py 91.66% <0%> (-2.78%) ⬇️
airflow/hooks/mysql_hook.py 90.38% <0%> (-2.6%) ⬇️
airflow/hooks/hive_hooks.py 73.42% <0%> (-1.85%) ⬇️
airflow/www_rbac/security.py 91.27% <0%> (-1.49%) ⬇️
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5035c1...22a7511. Read the comment docs.

@jeffkpayne

This comment has been minimized.

Copy link
Contributor

commented Oct 28, 2018

I'm probably not the best person to review this, having no experience with AWS, but one suggestion is adding some docstrings and maybe an example DAG. This PR is a great example: https://github.com/apache/incubator-airflow/pull/3969/files

@phani8996

This comment has been minimized.

Copy link
Contributor Author

commented Oct 28, 2018

I'm probably not the best person to review this, having no experience with AWS, but one suggestion is adding some docstrings and maybe an example DAG. This PR is a great example: https://github.com/apache/incubator-airflow/pull/3969/files

Doc part is still missing. I will be adding it next commit. I am more concerned about unit testing part. As current library does not support athena yet. I am hoping there is some way to test or some one can validate correctness and usability of above code.

@phani8996 phani8996 changed the title [AIRFLOW-3266] AWS Athena Operator and hook created [AIRFLOW-3266] Add AWS Athena Operator and hook Oct 29, 2018

@feluelle

This comment has been minimized.

Copy link
Contributor

commented Oct 29, 2018

Doc part is still missing. I will be adding it next commit. I am more concerned about unit testing part. As current library does not support athena yet. I am hoping there is some way to test or some one can validate correctness and usability of above code.

What you can always do in UNIT Tests is that you can mock the deps that means everything that does not belong to the responsibility of this Unit/Class you want to test.
That would mean you mock the functions of AwsHook you want to use.

@ron819

This comment has been minimized.

Copy link
Contributor

commented Oct 29, 2018

@feluelle

This comment has been minimized.

Copy link
Contributor

commented Oct 29, 2018

Shouldn't you also edit https://github.com/apache/incubator-airflow/blob/master/airflow/models.py#L752 and add your hook?

I think this is optional. Only if you want to have a custom conn_type for your hook.

@Fokko
Copy link
Contributor

left a comment

Thanks @phani8996 for contributing the Athena operator. I think this would be a nice addition to Airflow. Would it be possible to add tests using moto? Cheers!

airflow/contrib/hooks/aws_athena_hook.py Outdated
self.sleep_time = 5 # Use this to poll query status in athena
self.query_execution_context['Database'] = self.database
self.result_configuration['OutputLocation'] = self.outputLocation
super(AWSAthenaHook, self).__init__(self.aws_conn_id, verify=None)

This comment has been minimized.

Copy link
@Fokko

Fokko Oct 29, 2018

Contributor

Can we move this one up?

airflow/contrib/hooks/aws_athena_hook.py Outdated
self.region_name = region_name
self.outputLocation = output_location
self.aws_conn_id = aws_conn_id
if 'client_request_token' in kwargs: # This ensures idempotency of query execution

This comment has been minimized.

Copy link
@Fokko

Fokko Oct 29, 2018

Contributor

We could also simplify this and do it on a single line:

Suggested change
if 'client_request_token' in kwargs: # This ensures idempotency of query execution
self.client_request_token = kwargs.get('client_request_token') or {}
airflow/contrib/hooks/aws_athena_hook.py Outdated
self.conn = self.get_conn()

def _check_query_status(self):
if self._queryExecutionId is None:

This comment has been minimized.

Copy link
@Fokko

Fokko Oct 29, 2018

Contributor
Suggested change
if self._queryExecutionId is None:
if self._queryExecutionId is not None:
return self.conn.get_query_execution(QueryExecutionId=self._queryExecutionId)
airflow/contrib/hooks/aws_athena_hook.py Outdated
return self.conn.get_query_execution(QueryExecutionId=self._queryExecutionId)

def _get_query_results(self):
if self._queryExecutionId is None:

This comment has been minimized.

Copy link
@Fokko

Fokko Oct 29, 2018

Contributor

Same here, can we do if self._queryExecutionId is not None:. I think this easier to read.

airflow/contrib/hooks/aws_athena_hook.py Outdated

def _poll_query_status(self, sleep_time=None, max_tries=None):
try_number = 1
if sleep_time is None:

This comment has been minimized.

Copy link
@Fokko

Fokko Oct 29, 2018

Contributor

Please simplify this. We can just use the sleep_time from the self.sleep_time, and remove the sleep_time from the argument list.

@ashb ashb self-requested a review Oct 29, 2018

@ashb
Copy link
Member

left a comment

Thanks for picking this up - it will be a great addition.

But the API you've got is all wrong I'm afraid. The hook needs to be more general purpose. If I want to write a custom operator that runs a couple of queries, I should be able to do that with a single instance of the AthenaHook. (This was what I meant by my terse comment in Slack about these should all be non-private methods. That or they should live in the operator)

airflow/contrib/hooks/aws_athena_hook.py Outdated
def __init__(self, query, database, region_name, output_location, aws_conn_id=None, *args, **kwargs):
self.query = query
self.database = database
self.region_name = region_name

This comment has been minimized.

Copy link
@ashb

ashb Oct 29, 2018

Member

Please remove the region_name override - this was an older style of doing things ans we should instead take this from the extra dictionary of the connection.

airflow/contrib/hooks/aws_athena_hook.py Outdated
self.query_execution_context['Database'] = self.database
self.result_configuration['OutputLocation'] = self.outputLocation
super(AWSAthenaHook, self).__init__(self.aws_conn_id, verify=None)
self.conn = self.get_conn()

This comment has been minimized.

Copy link
@ashb

ashb Oct 29, 2018

Member

Please delay creating the connection until it is needed (i.e. not in the constructor)

airflow/contrib/hooks/aws_athena_hook.py Outdated
def poll_query_status(self, sleep_time=None, max_tries=None):
self._poll_query_status(sleep_time=None, max_tries=max_tries)

def on_kill(self):

This comment has been minimized.

Copy link
@ashb

ashb Oct 29, 2018

Member

On kill does not belong in the hook.

All of this logic needs to be in the operator.

airflow/contrib/hooks/aws_athena_hook.py Outdated
self.log.info("Received a kill Signal.")
self.log.info("Stopping Query with executionId - {queryId}".format(queryId=self._queryExecutionId))
response = self._stop_query()
HTTPStatusCode = None

This comment has been minimized.

Copy link
@ashb

ashb Oct 29, 2018

Member

Python style point: call thishttp_status_code

airflow/contrib/hooks/aws_athena_hook.py Outdated
class AWSAthenaHook(AwsHook):

def __init__(self, query, database, region_name, output_location, aws_conn_id=None, *args, **kwargs):
self.query = query

This comment has been minimized.

Copy link
@ashb

ashb Oct 29, 2018

Member

Query is not a property of a hook, this is part of the Operator API.

The hook should be reusable and not need a new hook for each query.

airflow/contrib/hooks/aws_athena_hook.py Outdated
if max_tries is None: # Retries forever until query reaches final state
retry_condition = True
else:
retry_condition = try_number <= max_tries

This comment has been minimized.

Copy link
@ashb

ashb Oct 29, 2018

Member

This doesn't work as you want/intend. This is evaluated one to true or false (almost always True) and then loops forever.

This comment has been minimized.

Copy link
@phani8996

phani8996 Oct 29, 2018

Author Contributor

@ashb This retry_condition variable will be evaluated at the end of each loop if max_retries is provided. This will run forever only when max_retries is not provided or else loop is gonna break after number of tries cross max_tries

This comment has been minimized.

Copy link
@ashb

ashb Oct 29, 2018

Member

Ah I missed that, sorry

Instead of evaluating the condition at the start (which would only fail if max_tries is 0) how about this:

while True:
    # checks

   if try_number > max_tries:
     break
     # Or raise exception etc.

This comment has been minimized.

Copy link
@ashb

ashb Oct 29, 2018

Member

Also: if it gets to the last try and it's still running it just silently fails I think? that should be an error condition.

This comment has been minimized.

Copy link
@phani8996

phani8996 Oct 29, 2018

Author Contributor

That's true @ashb. If query runs even after final state check, we can't do anything except to exit the function

airflow/contrib/operators/aws_athena_operator.py Outdated
self.hook.run_query()

def post_execute(self, context, result=None):
self.hook.poll_query_status()

This comment has been minimized.

Copy link
@ashb

ashb Oct 29, 2018

Member

What is the result parameter here for? Use it or remove it please.

airflow/contrib/operators/aws_athena_operator.py Outdated
template_fields = ('query', 'database', 'outputLocation', 'region_name')

@apply_defaults
def __init__(self, query, database, output_location, aws_conn_id=None, region_name=None, *args, **kwargs):

This comment has been minimized.

Copy link
@ashb

ashb Oct 29, 2018

Member

Default the connection to "aws_default" please.

@phani8996

This comment has been minimized.

Copy link
Contributor Author

commented Oct 29, 2018

@ashb, @Fokko thanks for the suggestion. It seems there are quite a few places where i took wrong assumptions on building operator and hook.

Based on suggestions i received, i want to break this into two parts.

  1. Build AWS athena hook with suggested changes as generic as possible to build multiple athena operators on top of it.
  2. Two Athena operators one for single query and other for batch queries.

I am removing Operator related code and will be cleaning/refactoring hook related code.

@phani8996 phani8996 force-pushed the phani8996:AIRFLOW-3266 branch 4 times, most recently Oct 29, 2018

airflow/contrib/operators/aws_athena_operator.py Outdated
self.hook.poll_query_status(self.query_execution_id)

def on_kill(self):
self.log.info('⚰️⚰️⚰️ Received a kill Signal. Time to Die.️')

This comment has been minimized.

Copy link
@XD-DENG

XD-DENG Oct 29, 2018

Member

The emoji is interesting. But it may not be displayed properly in the plaintext log later?

This comment has been minimized.

Copy link
@phani8996

phani8996 Oct 30, 2018

Author Contributor

This will be rendered as it is since our logging supports unicode. Please see attached screenshot of above log

img-20181030-wa0000
img-20181030-wa0001

@XD-DENG
Copy link
Member

left a comment

There is no docstring prepared for the AWSAthenaOperator added in this PR

@phani8996

This comment has been minimized.

Copy link
Contributor Author

commented Oct 30, 2018

@phani8996 phani8996 force-pushed the phani8996:AIRFLOW-3266 branch 4 times, most recently Oct 30, 2018

@ashb
Copy link
Member

left a comment

Thanks, the new structure is much better!

There is enough code in the hook's poll_query_status that I'd like to see that tested (right now it's just mocked out)

airflow/contrib/hooks/aws_athena_hook.py Outdated

def __init__(self, aws_conn_id='aws_default', *args, **kwargs):
super(AWSAthenaHook, self).__init__(aws_conn_id, **kwargs)
self.sleep_time = kwargs.get('sleep_time') or 30

This comment has been minimized.

Copy link
@ashb

ashb Oct 31, 2018

Member

This is an odd way of doing it - just add sleept_time=30 in to the __init__(...)

Show resolved Hide resolved airflow/contrib/hooks/aws_athena_hook.py
airflow/contrib/hooks/aws_athena_hook.py Outdated
:return: str
"""
if client_request_token is None:
client_request_token = str(uuid4())

This comment has been minimized.

Copy link
@ashb

ashb Oct 31, 2018

Member

Is this mandatory on the AWS API? Given this is randomly generated and not stored/returned anywhere is this useful? (I'm not familiar with the Athena APIs, so it may be)

This comment has been minimized.

Copy link
@phani8996

phani8996 Oct 31, 2018

Author Contributor

This essentially passed to avoid running same query multiple times.

This comment has been minimized.

Copy link
@ashb

ashb Oct 31, 2018

Member

I mean the generating a random string that is unknown to the caller.

This comment has been minimized.

Copy link
@phani8996

phani8996 Oct 31, 2018

Author Contributor

Got the point. I will move this logic to operator.

Show resolved Hide resolved airflow/contrib/hooks/aws_athena_hook.py
airflow/contrib/hooks/aws_athena_hook.py Outdated

INTERMEDIATE_STATES = ('QUEUED', 'RUNNING',)
FAILURE_STATES = ('FAILED', 'CANCELLED',)
SUCCESS_STATES = ('SUCCEEDED',)

This comment has been minimized.

Copy link
@ashb

ashb Oct 31, 2018

Member

Could you move these three to be properties on the class (so put them around line 33, after the class comment and before __init__ - this is for parity with other hooks/operators.

airflow/contrib/operators/aws_athena_operator.py Outdated
template_fields = ('query', 'database', 'output_location')

@apply_defaults
def __init__(self, query, database, output_location, aws_conn_id='aws_default', *args, **kwargs):

This comment has been minimized.

Copy link
@ashb

ashb Oct 31, 2018

Member

sleep_time should be configurable here too - 30s won't be long enough for some of the queries I'd like to run with this :)

airflow/contrib/operators/aws_athena_operator.py Outdated
"""
self.log.info('⚰️⚰️⚰️ Received a kill Signal. Time to Die')
self.log.info('Stopping Query with executionId - {queryId}'.format(queryId=self.query_execution_id))
response = self.hook.stop_query(self.query_execution_id)

This comment has been minimized.

Copy link
@ashb

ashb Oct 31, 2018

Member

It's theoretically possible that we get here before query_execution_id has been set/before we started running the query, so we should cope with that case

airflow/contrib/operators/aws_athena_operator.py Outdated
http_status_code = response['ResponseMetadata']['HTTPStatusCode']
finally:
if http_status_code is None or http_status_code != 200:
self.log.error('Invalid Stop request to Athena. Force killing the task')

This comment has been minimized.

Copy link
@ashb

ashb Oct 31, 2018

Member

This log talks about "Force killing task" but we don't do anything. I think the message just needs updating?

airflow/contrib/operators/aws_athena_operator.py Outdated
else:
self.log.info('Polling Athena for query with id {queryId} to reach final state'.format(
queryId=self.query_execution_id))
self.hook.poll_query_status(self.query_execution_id)

This comment has been minimized.

Copy link
@ashb

ashb Oct 31, 2018

Member

Is this wait useful? We've already asked AWS to stop the query, do we need to hang around waiting for it?

This comment has been minimized.

Copy link
@phani8996

phani8996 Oct 31, 2018

Author Contributor

Submitting a request does not essentially mean query is cancelled. In few cases, i have seen cancelling query takes more than a minute(for queries on huge data) to cancel. I feel like its better to wait until query is cancelled on athena

docs/integration.rst Outdated
AWSAthenaOperator
"""""""""""""""""""

.. autoclass:: airflow.contrib.operators.aws_athena_operator.AWSAthenaOperator

This comment has been minimized.

Copy link
@ashb

ashb Oct 31, 2018

Member

Oh, EMR is out of place here. Could you just add a link to your classes (hook and operator) in docs/code.rst instead.

(Don't remove EMR from this section, just don't make any changes to this file)

This comment has been minimized.

Copy link
@phani8996

phani8996 Oct 31, 2018

Author Contributor

I'm moving Athena operator doc to the bottom and adding the same on docs/code.rst

This comment has been minimized.

Copy link
@ashb

ashb Oct 31, 2018

Member

Please remove Athena from this section - EMR is out of place so we don't want to add more here.

tests/contrib/operators/test_aws_athena_operator.py Outdated

def test_init(self):
self.assertEqual(self.athena.task_id, MOCK_DATA['task_id'])
self.assertEqual(self.athena.query, MOCK_DATA['query'])
self.assertEqual(self.athena.database, MOCK_DATA['database'])
self.assertEqual(self.athena.aws_conn_id, 'aws_default')
self.assertEqual(self.athena.client_request_token, MOCK_DATA['client_request_token'])
self.assertEqual(self.athena.sleep_time, 1)

@mock.patch.object(AWSAthenaHook, 'check_query_status', side_effect=Iterator(("RUNNING", "SUCCESS",)))

This comment has been minimized.

Copy link
@ashb

ashb Nov 2, 2018

Member

Random-ness in tests seem like a bad plan at first sight.

Can you explain your reasoning for doing this, rather than say just side_effect=["RUNNING", "SUCCESS"] which could return running first time and success the second time. Right now with randomness we don't know how many times it might have to check the query otherwise, or am I missing something?

This comment has been minimized.

Copy link
@phani8996

phani8996 Nov 2, 2018

Author Contributor

At first i thought of having side_effect=("RUNNING", "SUCCESS",), but later i realised in a real query, task won't reach state SUCCESS in single sleep cycle. Randomly picking state sounds little odd but for our case, it will serve the process. We will get multiple RUNNING states and after few sleep cycles we will encounter SUCCESS state and poll_query_results will return the final state. To make this more understandable we can add bias to state RUNNING and make it look realistic. Returning SUCCESS in first check symbolises a fast query and SUCCESS after few checks symbolises long running queries which we cannot achieve with plain side_effect=("RUNNING", "SUCCESS",)

This comment has been minimized.

Copy link
@ashb

ashb Nov 2, 2018

Member

How about two checks/calls one that has side_effect=("RUNNING", "RUNNING", "RUNNING", "SUCCESS",) and a second that does side_effect= "SUCCESS",)?

Or at very least capture your reasoning in a comment in the code :)

This comment has been minimized.

Copy link
@phani8996

phani8996 Nov 2, 2018

Author Contributor

I will add multiple test cases for short and long running queries. Explaining this whole logic as comment will be little odd :P

@phani8996 phani8996 force-pushed the phani8996:AIRFLOW-3266 branch 2 times, most recently Nov 2, 2018

@phani8996

This comment has been minimized.

Copy link
Contributor Author

commented Nov 6, 2018

@ashb requested changes have been made. Please review and share if anything else need to be done.

@Fokko

Fokko approved these changes Nov 13, 2018

@ashb

ashb approved these changes Nov 13, 2018

Copy link
Member

left a comment

Nice!

@phani8996 phani8996 force-pushed the phani8996:AIRFLOW-3266 branch Nov 13, 2018

@phani8996

This comment has been minimized.

Copy link
Contributor Author

commented Nov 13, 2018

@ashb @Fokko Thanks for guiding me through this. I am squashing my commits. Please merge this PR.

@phani8996

This comment has been minimized.

Copy link
Contributor Author

commented Nov 14, 2018

@ashb @Fokko Thanks for guiding me through this. I am squashing my commits. Please merge this PR.

Can someone please merge it. All the tests are passed, code is approved and all my commits are squashed.

@ashb

This comment has been minimized.

Copy link
Member

commented Nov 14, 2018

@phani8996 One (last, I promise!) question: have you run this code yet in a DAG or via airflow test etc to check that everything works as expected? Unit tests are great but they don't catch everything.

[AIRFLOW-3266] Add AWS Athena Hook and Operator
Provides AWS Athena hook and operator to submit athena(presto) queries on AWS.

Authored-by: Phanindhra <phani8996@gmail.com>
@phani8996

This comment has been minimized.

Copy link
Contributor Author

commented Nov 15, 2018

@phani8996 One (last, I promise!) question: have you run this code yet in a DAG or via airflow test etc to check that everything works as expected? Unit tests are great but they don't catch everything.

@ashb I moved above code to our production airflow cluster and its working as expected in cases of success queries, failed queries, invalid queries.
Here is log snippet of success query.

Log file does not exist: /var/log/airflow/trains.test_dag/repair_task/2018-11-15T02:49:08.016673+00:00/1.log
Fetching from: http://<airflow_ip>:<airflow_port>/log/trains.test_dag/repair_task/2018-11-15T02:49:08.016673+00:00/1.log?num_lines=300

Showing only last 300 lines from trains.test_dag/repair_task/2018-11-15T02:49:08.016673+00:00/1.log


Starting attempt 1 of 2

[2018-11-15 02:49:11,350] INFO - Executing <Task(AWSAthenaOperator): repair_task> on 2018-11-15T02:49:08.016673+00:00
[2018-11-15 02:49:11,350] INFO - Running: ['bash', '-c', 'airflow run trains.test_dag repair_task 2018-11-15T02:49:08.016673+00:00 --job_id 29420 --pool default --raw -sd /mnt/src/airflow-ds/dags/trains/test_dag.py --cfg_path /tmp/tmpeb_ttzdb']
[2018-11-15 02:49:11,798] INFO - Job 29420: Subtask repair_task [2018-11-15 02:49:11,798] INFO - setting.configure_orm(): Using pool settings. pool_size=5, pool_recycle=1800
[2018-11-15 02:49:12,257] INFO - Job 29420: Subtask repair_task [2018-11-15 02:49:12,256] INFO - Using executor CeleryExecutor
[2018-11-15 02:49:12,384] INFO - Job 29420: Subtask repair_task [2018-11-15 02:49:12,384] INFO - Filling up the DagBag from /mnt/src/airflow-ds/dags/trains/test_dag.py
[2018-11-15 02:49:12,668] INFO - Job 29420: Subtask repair_task [2018-11-15 02:49:12,668] INFO - Running <TaskInstance: trains.test_dag.repair_task 2018-11-15T02:49:08.016673+00:00 [running]> on host <airflow_worker_ip>
[2018-11-15 02:49:12,936] INFO - [2018-11-15 02:49:12,936] INFO - Trial 1: Query is still in an intermediate state - RUNNING

[2018-11-15 02:49:42,988] INFO - [2018-11-15 02:49:42,987] INFO - Trial 2: Query execution completed. Final state is SUCCEEDED

[2018-11-15 02:50:11,456] INFO - [2018-11-15 02:50:11,455] INFO - Task exited with return code 0

@phani8996 phani8996 force-pushed the phani8996:AIRFLOW-3266 branch to 22a7511 Nov 15, 2018

@phani8996

This comment has been minimized.

Copy link
Contributor Author

commented Nov 15, 2018

@ashb @kaxil Can you please review these final changes.

@kaxil

kaxil approved these changes Nov 15, 2018

@kaxil

This comment has been minimized.

Copy link
Contributor

commented Nov 15, 2018

@ashb Any comments?

@ashb

This comment has been minimized.

Copy link
Member

commented Nov 16, 2018

Assuming it was just formatting changes since I last looked LGTM.

@phani8996

This comment has been minimized.

Copy link
Contributor Author

commented Nov 16, 2018

Assuming it was just formatting changes since I last looked LGTM.

Changes are new line in docstring. Apart from that everything else is same.

@ashb ashb merged commit 7d23dd8 into apache:master Nov 16, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@phani8996

This comment has been minimized.

Copy link
Contributor Author

commented Nov 16, 2018

Thank you very much @ashb for guided me through the right path. Thanks @Fokko @kaxil @ckljohn @XD-DENG for reviewing this PR.

tmiller-msft added a commit to cse-airflow/incubator-airflow that referenced this pull request Nov 27, 2018

[AIRFLOW-3266] Add AWS Athena Hook and Operator (apache#4111)
Provides AWS Athena hook and operator to submit Athena(presto) queries on AWS.

Authored-by: Phanindhra <phani8996@gmail.com>

kaxil added a commit that referenced this pull request Dec 2, 2018

[AIRFLOW-3266] Add AWS Athena Hook and Operator (#4111)
Provides AWS Athena hook and operator to submit Athena(presto) queries on AWS.

Authored-by: Phanindhra <phani8996@gmail.com>

aliceabe pushed a commit to aliceabe/incubator-airflow that referenced this pull request Jan 3, 2019

[AIRFLOW-3266] Add AWS Athena Hook and Operator (apache#4111)
Provides AWS Athena hook and operator to submit Athena(presto) queries on AWS.

Authored-by: Phanindhra <phani8996@gmail.com>

kaxil added a commit that referenced this pull request Jan 9, 2019

[AIRFLOW-3266] Add AWS Athena Hook and Operator (#4111)
Provides AWS Athena hook and operator to submit Athena(presto) queries on AWS.

Authored-by: Phanindhra <phani8996@gmail.com>

ashb added a commit to ashb/airflow that referenced this pull request Jan 10, 2019

[AIRFLOW-3266] Add AWS Athena Hook and Operator (apache#4111)
Provides AWS Athena hook and operator to submit Athena(presto) queries on AWS.

Authored-by: Phanindhra <phani8996@gmail.com>

cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Jan 23, 2019

Add AWS Athena Hook and Operator (apache#4111)
Provides AWS Athena hook and operator to submit Athena(presto) queries on AWS.

Authored-by: Phanindhra <phani8996@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.