potiuk commented on code in PR #30308:
URL: https://github.com/apache/airflow/pull/30308#discussion_r1162527559


##########
airflow/jobs/job.py:
##########
@@ -292,3 +281,51 @@ def most_recent_job(job_type: str, session: Session = 
NEW_SESSION) -> Job | None
         )
         .first()
     )
+
+
+@provide_session
+def prepare_for_execution(job: Job | JobPydantic, session: Session = 
NEW_SESSION):

Review Comment:
   Hmm. I tried to do some type conditionals around it. We need to be able to 
support it also in the near future when we start getting the AIP-44 variants.
   
   But this does not really work:
   ```
   if TYPE_CHECKING:
       if _ENABLE_AIP_44:
           JobType = TypeVar("JobType", Job, JobPydantic)
       else:
           JobType = TypeVar("JobType", bound=Job)
   ```
   
   Produces:
   
   
   ```
   airflow/jobs/job.py:251: error: Cannot redefine "JobType" as a type variable
   [misc]
               JobType = TypeVar("JobType", bound=Job)
               ^
   airflow/jobs/job.py:251: error: Invalid assignment target  [misc]
               JobType = TypeVar("JobType", bound=Job)
               ^
   ```
   
   Maybe you can have idea how to do both better - you have more experience 
with complex and conditional types @uranusjr ? If we can't get it easy, I think 
doing the instance check is a small price to pay for having it "simpler". 



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