[GitHub] [airflow] uranusjr commented on a diff in pull request #24912: More typing in SchedulerJob and TaskInstance

[GitBox <gi...@apache.org>]

uranusjr commented on code in PR #24912:
URL: https://github.com/apache/airflow/pull/24912#discussion_r916450273


##########
airflow/models/taskinstance.py:
##########
@@ -163,12 +165,12 @@ def set_current_context(context: Context) -> Generator[Context, None, None]:
 
 
 def clear_task_instances(
-    tis,
-    session,
+    tis: List["TaskInstance"],
+    session: Session,
     activate_dag_runs=None,
     dag=None,

Review Comment:
   ```suggestion
       activate_dag_runs: None = None,
       dag: Optional[DAG] = None,
   ```



##########
airflow/models/taskinstance.py:
##########
@@ -790,11 +791,11 @@ def log_url(self):
         )
 
     @property
-    def mark_success_url(self):
+    def mark_success_url(self) -> str:
         """URL to mark TI success"""
         base_url = conf.get('webserver', 'BASE_URL')
-        return base_url + (
-            "/confirm"
+        return (
+            f"{base_url}/confirm"

Review Comment:
   Should we use `get_mandatory_value` instead?



##########
airflow/jobs/scheduler_job.py:
##########
@@ -1194,7 +1195,7 @@ def _schedule_dag_run(
         return callback_to_run
 
     @provide_session
-    def _verify_integrity_if_dag_changed(self, dag_run: DagRun, session=None):
+    def _verify_integrity_if_dag_changed(self, dag_run: DagRun, session: Session = None) -> None:

Review Comment:
   Use `NEW_SESSION` here



-- 
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: commits-unsubscribe@airflow.apache.org

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