You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/05/27 03:33:05 UTC

[GitHub] [airflow] ljb7977 opened a new pull request #9022: Fix AwsGlueJobSensor

ljb7977 opened a new pull request #9022:
URL: https://github.com/apache/airflow/pull/9022


   ---
   Make sure to mark the boxes below before creating PR: [x]
   
   - [ ] Description above provides context of the change
   - [ ] Unit tests coverage for changes (not needed for documentation changes)
   - [ ] Target Github ISSUE in description if exists
   - [ ] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
   - [ ] Relevant documentation is updated including usage instructions.
   - [ ] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
   
   ---
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
   Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
   
   Related: https://github.com/apache/airflow/issues/9021


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ljb7977 edited a comment on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
ljb7977 edited a comment on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-669986197


   > @ljb7977 how is this going? Could you please also do a rebase together with the changes? :)
   
   Sorry for late replying. I made changes that you suggested and fixed some tests.
   Question: Is it good to allow AwsGlueJobHook to have context about glue job? I mean, can the hook keep run_id of the glue job it invoked or should it be static?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] feluelle commented on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
feluelle commented on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-675373201


   @ljb7977 no problem. :) 
   
   Thank you for contributing! :) Looking forward to further contributions :)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ljb7977 edited a comment on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
ljb7977 edited a comment on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-669986197


   > @ljb7977 how is this going? Could you please also do a rebase together with the changes? :)
   
   @feluelle Sorry for late replying. I made changes that you suggested and fixed some tests.
   Question: Is it good to allow AwsGlueJobHook to have context about glue job? I mean, can the hook keep run_id of the glue job it invoked or should it be static?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ljb7977 commented on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
ljb7977 commented on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-675349912


   Oh I checked just now that this is merged. Sorry that I firstly missed to add not much context when I created this PR. I was not familiar with contributing to an open source project. Thank you for understanding and accepting the PR!
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] feluelle commented on a change in pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#discussion_r454163716



##########
File path: airflow/providers/amazon/aws/sensors/glue.py
##########
@@ -47,14 +47,13 @@ def __init__(self,
         self.aws_conn_id = aws_conn_id
         self.success_states = ['SUCCEEDED']
         self.errored_states = ['FAILED', 'STOPPED', 'TIMEOUT']
+        self.hook = AwsGlueJobHook(aws_conn_id=self.aws_conn_id)

Review comment:
       A hook should not be initialzed during operator/sensor init, because it will be called everytime the webserver parses the dag file the operator is initiated (`task = AwsGlueJobSensor(...)`). The hook initialisation sometimes contains connections to external services which would slow down the Airflow instance.
   You only need the hook object in the `poke`/`execute` function when you actually run the task.

##########
File path: airflow/providers/amazon/aws/hooks/glue.py
##########
@@ -110,24 +110,36 @@ def initialize_job(self, script_arguments=None):
             self.log.error("Failed to run aws glue job, error: %s", general_error)
             raise
 
-    def job_completion(self, job_name=None, run_id=None):
+    def get_job_state(self, job_name=None, run_id=None):
         """
         :param job_name: unique job name per AWS account
         :type job_name: str
         :param run_id: The job-run ID of the predecessor job run
         :type run_id: str
         :return: Status of the Job if succeeded or stopped
         """
+        glue_client = self.get_conn()
+        job_status = glue_client.get_job_run(
+            JobName=job_name,
+            RunId=run_id,
+            PredecessorsIncluded=True
+        )
+        job_run_state = job_status['JobRun']['JobRunState']
+        return job_run_state
+
+    def job_completion(self, job_name=None, run_id=None):
+        """
+        :param job_name: unique job name per AWS account

Review comment:
       Same here, please :)

##########
File path: airflow/providers/amazon/aws/hooks/glue.py
##########
@@ -110,24 +110,36 @@ def initialize_job(self, script_arguments=None):
             self.log.error("Failed to run aws glue job, error: %s", general_error)
             raise
 
-    def job_completion(self, job_name=None, run_id=None):
+    def get_job_state(self, job_name=None, run_id=None):
         """
         :param job_name: unique job name per AWS account

Review comment:
       Please also add a short title / description what this function does. :)

##########
File path: airflow/providers/amazon/aws/hooks/glue.py
##########
@@ -110,24 +110,36 @@ def initialize_job(self, script_arguments=None):
             self.log.error("Failed to run aws glue job, error: %s", general_error)
             raise
 
-    def job_completion(self, job_name=None, run_id=None):
+    def get_job_state(self, job_name=None, run_id=None):
         """
         :param job_name: unique job name per AWS account
         :type job_name: str
         :param run_id: The job-run ID of the predecessor job run
         :type run_id: str
         :return: Status of the Job if succeeded or stopped

Review comment:
       That is not correct here. `get_job_state` can return any state?!




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ljb7977 commented on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
ljb7977 commented on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-669986197


   > @ljb7977 how is this going? Could you please also do a rebase together with the changes? :)
   
   Sorry I'm late. I made changes that you suggested and fixed some tests.
   Question: Is it good to allow AwsGlueJobHook to have context about glue job? I mean, can the hook keep run_id of the glue job it invoked or should it be static?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ljb7977 edited a comment on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
ljb7977 edited a comment on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-675349912


   Oh I checked just now that this is merged.
   Sorry that I firstly missed to add not much context when I created this PR. I was not familiar with contributing to an open source project.
   Thank you for understanding and accepting the PR! πŸ™ 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] feluelle commented on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
feluelle commented on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-674981612


   ..and description was also there. But I also missed it at first. :) I moved it to the top as well.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] boring-cyborg[bot] commented on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-634070516


   Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
   Here are some useful points:
   - Pay attention to the quality of your code (flake8, pylint and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/master/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
   - In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/master/docs/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
   - Consider using [Breeze environment](https://github.com/apache/airflow/blob/master/BREEZE.rst) for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
   - Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
   - Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#coding-style-and-best-practices).
   Apache Airflow is a community-driven project and together we are making it better πŸš€.
   In case of doubts contact the developers at:
   Mailing List: dev@airflow.apache.org
   Slack: https://apache-airflow-slack.herokuapp.com/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ljb7977 commented on a change in pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
ljb7977 commented on a change in pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#discussion_r466430388



##########
File path: airflow/providers/amazon/aws/hooks/glue.py
##########
@@ -110,24 +110,36 @@ def initialize_job(self, script_arguments=None):
             self.log.error("Failed to run aws glue job, error: %s", general_error)
             raise
 
-    def job_completion(self, job_name=None, run_id=None):
+    def get_job_state(self, job_name=None, run_id=None):
         """
         :param job_name: unique job name per AWS account
         :type job_name: str
         :param run_id: The job-run ID of the predecessor job run
         :type run_id: str
         :return: Status of the Job if succeeded or stopped

Review comment:
       Right. It can return any state of actual Glue job can have so I'm updating the description.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] feluelle commented on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
feluelle commented on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-666229509


   @ljb7977 how is this going? Could you please also do a rebase together with the changes? :)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ashb commented on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-653878180


   Please describe the problem this PR solves


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] feluelle edited a comment on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
feluelle edited a comment on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-674982709


   But how about commit title: _Fix AwsGlueJobSensor to stop running after the Glue job finished_
   
   (I was about to merge it with this change while you were reviewing it)
   
   It LGTM.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] feluelle commented on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
feluelle commented on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-674982709


   But how about commit title: _Fix AwsGlueJobSensor to stop running after the Glue job finished_


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-674981277


   ah right :)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ljb7977 commented on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
ljb7977 commented on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-661159529


   @feluelle Thank you for the review! I'll make changes in a few days.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] boring-cyborg[bot] commented on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-674987736


   Awesome work, congrats on your first merged pull request!
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ljb7977 commented on a change in pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
ljb7977 commented on a change in pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#discussion_r466431912



##########
File path: airflow/providers/amazon/aws/hooks/glue.py
##########
@@ -110,24 +110,36 @@ def initialize_job(self, script_arguments=None):
             self.log.error("Failed to run aws glue job, error: %s", general_error)
             raise
 
-    def job_completion(self, job_name=None, run_id=None):
+    def get_job_state(self, job_name=None, run_id=None):
         """
         :param job_name: unique job name per AWS account
         :type job_name: str
         :param run_id: The job-run ID of the predecessor job run
         :type run_id: str
         :return: Status of the Job if succeeded or stopped
         """
+        glue_client = self.get_conn()
+        job_status = glue_client.get_job_run(
+            JobName=job_name,
+            RunId=run_id,
+            PredecessorsIncluded=True
+        )
+        job_run_state = job_status['JobRun']['JobRunState']
+        return job_run_state
+
+    def job_completion(self, job_name=None, run_id=None):
+        """
+        :param job_name: unique job name per AWS account

Review comment:
       πŸ™†β€β™‚οΈ 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] feluelle commented on a change in pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#discussion_r470836977



##########
File path: airflow/providers/amazon/aws/hooks/glue.py
##########
@@ -105,29 +105,46 @@ def initialize_job(self, script_arguments=None):
                 JobName=job_name,
                 Arguments=script_arguments
             )
-            return self.job_completion(job_name, job_run['JobRunId'])
+            return job_run
         except Exception as general_error:
             self.log.error("Failed to run aws glue job, error: %s", general_error)
             raise
 
+    def get_job_state(self, job_name=None, run_id=None):
+        """
+        Get state of the Glue job. The job state can be
+        running, finished, failed, stopped or timeout.
+        :param job_name: unique job name per AWS account
+        :type job_name: str
+        :param run_id: The job-run ID of the predecessor job run
+        :type run_id: str
+        :return: State of the Glue job
+        """
+        glue_client = self.get_conn()
+        job_status = glue_client.get_job_run(
+            JobName=job_name,
+            RunId=run_id,
+            PredecessorsIncluded=True
+        )
+        job_run_state = job_status['JobRun']['JobRunState']
+        return job_run_state

Review comment:
       ```suggestion
           job_run = glue_client.get_job_run(
               JobName=job_name,
               RunId=run_id,
               PredecessorsIncluded=True
           )
           job_run_state = job_run['JobRun']['JobRunState']
           return job_run_state
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] feluelle merged pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
feluelle merged pull request #9022:
URL: https://github.com/apache/airflow/pull/9022


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-674982037


   Shall we merge then :) @feluelle ? 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] feluelle commented on a change in pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
feluelle commented on a change in pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#discussion_r470836977



##########
File path: airflow/providers/amazon/aws/hooks/glue.py
##########
@@ -105,29 +105,46 @@ def initialize_job(self, script_arguments=None):
                 JobName=job_name,
                 Arguments=script_arguments
             )
-            return self.job_completion(job_name, job_run['JobRunId'])
+            return job_run
         except Exception as general_error:
             self.log.error("Failed to run aws glue job, error: %s", general_error)
             raise
 
+    def get_job_state(self, job_name=None, run_id=None):
+        """
+        Get state of the Glue job. The job state can be
+        running, finished, failed, stopped or timeout.
+        :param job_name: unique job name per AWS account
+        :type job_name: str
+        :param run_id: The job-run ID of the predecessor job run
+        :type run_id: str
+        :return: State of the Glue job
+        """
+        glue_client = self.get_conn()
+        job_status = glue_client.get_job_run(
+            JobName=job_name,
+            RunId=run_id,
+            PredecessorsIncluded=True
+        )
+        job_run_state = job_status['JobRun']['JobRunState']
+        return job_run_state

Review comment:
       ```suggestion
           job_run = glue_client.get_job_run(
               JobName=job_name,
               RunId=run_id,
               PredecessorsIncluded=True
           )
           return job_run['JobRunState']
   ```
   Shouldn't it be like this?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] feluelle commented on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
feluelle commented on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-674979701


   @potiuk I moved:
   
   > Related: https://github.com/apache/airflow/issues/9021
   
   to the top. It explains it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] ljb7977 commented on a change in pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
ljb7977 commented on a change in pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#discussion_r466431481



##########
File path: airflow/providers/amazon/aws/sensors/glue.py
##########
@@ -47,14 +47,13 @@ def __init__(self,
         self.aws_conn_id = aws_conn_id
         self.success_states = ['SUCCEEDED']
         self.errored_states = ['FAILED', 'STOPPED', 'TIMEOUT']
+        self.hook = AwsGlueJobHook(aws_conn_id=self.aws_conn_id)

Review comment:
       Got it. I misunderstood how __init__ of sensor works. It's fixed now :)

##########
File path: airflow/providers/amazon/aws/sensors/glue.py
##########
@@ -47,14 +47,13 @@ def __init__(self,
         self.aws_conn_id = aws_conn_id
         self.success_states = ['SUCCEEDED']
         self.errored_states = ['FAILED', 'STOPPED', 'TIMEOUT']
+        self.hook = AwsGlueJobHook(aws_conn_id=self.aws_conn_id)

Review comment:
       Got it. I've misunderstood how __init__ of sensor works. It's fixed now :)

##########
File path: airflow/providers/amazon/aws/sensors/glue.py
##########
@@ -47,14 +47,13 @@ def __init__(self,
         self.aws_conn_id = aws_conn_id
         self.success_states = ['SUCCEEDED']
         self.errored_states = ['FAILED', 'STOPPED', 'TIMEOUT']
+        self.hook = AwsGlueJobHook(aws_conn_id=self.aws_conn_id)

Review comment:
       Got it. I've misunderstood how `__init__` of sensor works. It's fixed now :)

##########
File path: airflow/providers/amazon/aws/sensors/glue.py
##########
@@ -47,14 +47,13 @@ def __init__(self,
         self.aws_conn_id = aws_conn_id
         self.success_states = ['SUCCEEDED']
         self.errored_states = ['FAILED', 'STOPPED', 'TIMEOUT']
+        self.hook = AwsGlueJobHook(aws_conn_id=self.aws_conn_id)

Review comment:
       Got it. I've misunderstood how `__init__` of a sensor works. It's fixed now :)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [airflow] potiuk commented on pull request #9022: Fix AwsGlueJobSensor

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #9022:
URL: https://github.com/apache/airflow/pull/9022#issuecomment-674977315


   @ljb7977 -> I am almost happy with the PR and would love to merge it, but I would love if you provide a bit more context on the error fixed? A paragraph explaining it maybe ?
   
   There is no need to change the commit or rebase - if you add the paragraph as a comment here, I can add it at merge time.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org