o-nikolas commented on code in PR #30162:
URL: https://github.com/apache/airflow/pull/30162#discussion_r1142575275


##########
airflow/providers/amazon/aws/hooks/glue.py:
##########
@@ -162,8 +165,11 @@ def initialize_job(
         run_kwargs = run_kwargs or {}
 
         try:
-            job_name = self.create_or_update_glue_job()
-            return self.get_conn().start_job_run(JobName=job_name, 
Arguments=script_arguments, **run_kwargs)
+            if self.update_config:
+                self.create_or_update_glue_job()
+            return self.get_conn().start_job_run(

Review Comment:
   Thanks for the fast turnaround!
   
   I don't think this quite matches the behaviour before your config update 
changes. Previously the code would check for an existing job with that name and 
then start it OR it would create the job if it didn't find an existing one, and 
then start that one. In this PR it seems if `update_config` is False (the 
default) we go straight to starting the job without first creating it if it 
doesn't exit (but I may be missing something).
   I think you want something like:
   ```Python
   try:
       if self.update_config:
           job_name = self.create_or_update_glue_job()
       else:
           job_name = self.get_or_create_glue_job()
   
       return glue_client.start_job_run(JobName=job_name, 
Arguments=script_arguments, **run_kwargs)
   
   except Exception as general_error:
       self.log.error("Failed to run aws glue job, error: %s", general_error)
       raise
   ```



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to