[GitHub] [airflow] jedcunningham commented on a diff in pull request #29714: Add database check_migrations config

["jedcunningham (via GitHub)" <gi...@apache.org>]

jedcunningham commented on code in PR #29714:
URL: https://github.com/apache/airflow/pull/29714#discussion_r1117792713


##########
tests/cli/commands/test_scheduler_command.py:
##########
@@ -74,6 +74,38 @@ def test_skip_serve_logs(self, mock_process, mock_scheduler_job, executor):
             with pytest.raises(AssertionError):
                 mock_process.assert_has_calls([mock.call(target=serve_logs)])
 
+    @mock.patch("airflow.utils.db.check_and_run_migrations")
+    @mock.patch("airflow.utils.db.synchronize_log_template")
+    @mock.patch("airflow.cli.commands.scheduler_command.SchedulerJob")
+    @mock.patch("airflow.cli.commands.scheduler_command.Process")
+    @pytest.mark.parametrize("executor", ["LocalExecutor"])
+    def test_check_migrations_is_false(
+        self, mock_process, mock_scheduler_job, mock_log, mock_run_migration, executor

Review Comment:
   ```suggestion
       def test_check_migrations_is_false(
           self, mock_process, mock_scheduler_job, mock_log, mock_run_migration
   ```
   
   You aren't using this, so let's get rid of it.



##########
tests/cli/commands/test_scheduler_command.py:
##########
@@ -74,6 +74,38 @@ def test_skip_serve_logs(self, mock_process, mock_scheduler_job, executor):
             with pytest.raises(AssertionError):
                 mock_process.assert_has_calls([mock.call(target=serve_logs)])
 
+    @mock.patch("airflow.utils.db.check_and_run_migrations")
+    @mock.patch("airflow.utils.db.synchronize_log_template")
+    @mock.patch("airflow.cli.commands.scheduler_command.SchedulerJob")
+    @mock.patch("airflow.cli.commands.scheduler_command.Process")
+    @pytest.mark.parametrize("executor", ["LocalExecutor"])
+    def test_check_migrations_is_false(
+        self, mock_process, mock_scheduler_job, mock_log, mock_run_migration, executor
+    ):
+        args = self.parser.parse_args(["scheduler"])
+        with conf_vars(
+            {("core", "executor"): "SequentialExecutor", ("database", "check_migrations"): "False"}

Review Comment:
   ```suggestion
               {("database", "check_migrations"): "False"}
   ```
   
   Is the executor config relevant 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