ferruzzi commented on code in PR #32604:
URL: https://github.com/apache/airflow/pull/32604#discussion_r1265768024
##########
TESTING.rst:
##########
@@ -55,8 +55,33 @@ Follow the guidelines when writing unit tests:
tests, so we run Pytest with ``--disable-warnings`` but instead we have
``pytest-capture-warnings`` plugin that
overrides ``recwarn`` fixture behaviour.
-**NOTE:** We plan to convert all unit tests to standard "asserts"
semi-automatically, but this will be done later
-in Airflow 2.0 development phase. That will include setUp/tearDown/context
managers and decorators.
+
+.. note::
+
+ We are in the process of convert all unit tests to standard "asserts" and
pytest fixtures
+ so if you find some tests that are still using classic setUp/tearDown
approach or unittest asserts, feel
+ free to convert them to pytest.
+
+Airflow configuration for unit tests
+------------------------------------
+
+Some of the unit tests require special configuration set as "default" one.
This is done automatically by
+automatically adding ``AIRFLOW__CORE__UNIT_TEST_MODE=True`` to the environment
variables in Pytest auto-used
+fixture. This in turn makes airflow configuration load test configuration from
+``airflow/config_templates/config_unit_test.yml`` - defaults from the test
configuration replace the original
+defaults stored in ``airflow/config_templates/config.yml``. If you want to add
some test-only configuration,
+you should copy the option definition to ``config_unit_test.yml`` and change
default value there.
Review Comment:
Lots of repetition makes this hard to parse. I'm going to attempt to
rephrase it, feel free to take it or now.
```suggestion
Some unit tests require special configurations set as "default". A Pytest
auto-use fixture automatically adds ``AIRFLOW__CORE__UNIT_TEST_MODE=True`` to
the environment variables to handle this. This, in turn, loads the test's
configuration from
``airflow/config_templates/config_unit_test.yml``. Defaults from the test
configuration replace the original
defaults stored in ``airflow/config_templates/config.yml``. If you want to
add some test-only configuration,
you should copy the option definition to ``config_unit_test.yml`` and change
default values there.
```
##########
TESTING.rst:
##########
@@ -55,8 +55,33 @@ Follow the guidelines when writing unit tests:
tests, so we run Pytest with ``--disable-warnings`` but instead we have
``pytest-capture-warnings`` plugin that
overrides ``recwarn`` fixture behaviour.
-**NOTE:** We plan to convert all unit tests to standard "asserts"
semi-automatically, but this will be done later
-in Airflow 2.0 development phase. That will include setUp/tearDown/context
managers and decorators.
+
+.. note::
+
+ We are in the process of convert all unit tests to standard "asserts" and
pytest fixtures
+ so if you find some tests that are still using classic setUp/tearDown
approach or unittest asserts, feel
+ free to convert them to pytest.
+
+Airflow configuration for unit tests
+------------------------------------
+
+Some of the unit tests require special configuration set as "default" one.
This is done automatically by
+automatically adding ``AIRFLOW__CORE__UNIT_TEST_MODE=True`` to the environment
variables in Pytest auto-used
+fixture. This in turn makes airflow configuration load test configuration from
+``airflow/config_templates/config_unit_test.yml`` - defaults from the test
configuration replace the original
+defaults stored in ``airflow/config_templates/config.yml``. If you want to add
some test-only configuration,
+you should copy the option definition to ``config_unit_test.yml`` and change
default value there.
+
+.. note::
+
+ The test configuration for Airflow before July 2023 was automatically
generated in a file named
+ ``AIRFLOW_HOME/unittest.cfg`` and template for it was stored in
"config_templates" next to the yaml file,
+ however this was only done for the first time you run airflow and you had to
manually maintain the file,
+ and it was pretty cumbrsome and arcane knowledge, also this file has been
overwritten in Breeze environment
+ with another CI-specific file. Using ````config_unit_test.yml`` as a single
source of configuration for
+ tests is much more convenient and it is automatically used by Pytest. If you
have the automatically
+ generated ``unittest.cfg`` file it will be automatically removed when you
run airflow.
+
Review Comment:
```suggestion
The test configuration for Airflow before July 2023 was automatically
generated in a file named
``AIRFLOW_HOME/unittest.cfg`` and the template for it was stored in
"config_templates" next to the yaml file. This was only done for the first time
you run Airflow and you had to manually maintain the file, which was pretty
cumbersome and arcane knowledge. This file has been overwritten in the Breeze
environment with another CI-specific file. Using ````config_unit_test.yml`` as
a single source of configuration for tests is much more convenient and it is
automatically used by Pytest. If you have the automatically generated
``unittest.cfg`` file it will be automatically removed when you run Airflow.
```
##########
airflow/config_templates/config_unit_tests.yml:
##########
@@ -0,0 +1,221 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Defaults specified in this file replace the default values from
+# config.yml file during unit tests. If you want a value to have
+# different default during unit tests, specify it here.
+
+---
+
+core:
+ description: ~
+ options:
+ dags_folder:
+ description: |
+ The folder where your airflow pipelines live, most likely a
+ subfolder in a code repository. This path must be absolute.
+ version_added: ~
+ type: string
+ example: ~
+ default: "{AIRFLOW_HOME}/dags"
+ executor:
+ description: |
+ The executor class that airflow should use. Choices include
Review Comment:
```suggestion
The executor class that Airflow should use. Choices include
```
##########
airflow/config_templates/config_unit_tests.yml:
##########
@@ -0,0 +1,221 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Defaults specified in this file replace the default values from
+# config.yml file during unit tests. If you want a value to have
+# different default during unit tests, specify it here.
+
+---
+
+core:
+ description: ~
+ options:
+ dags_folder:
+ description: |
+ The folder where your airflow pipelines live, most likely a
+ subfolder in a code repository. This path must be absolute.
+ version_added: ~
+ type: string
+ example: ~
+ default: "{AIRFLOW_HOME}/dags"
+ executor:
+ description: |
+ The executor class that airflow should use. Choices include
+ ``SequentialExecutor``, ``LocalExecutor``, ``CeleryExecutor``,
``DaskExecutor``,
+ ``KubernetesExecutor``, ``CeleryKubernetesExecutor`` or the
+ full import path to the class when using a custom executor.
+ version_added: ~
+ type: string
+ example: ~
+ default: "LocalExecutor"
+ dags_are_paused_at_creation:
+ description: |
+ Are DAGs paused by default at creation
+ version_added: ~
+ type: string
+ example: ~
+ default: "False"
+ load_examples:
+ description: |
+ Whether to load the DAG examples that ship with Airflow. It's good to
+ get started, but you probably want to set this to ``False`` in a
production
+ environment
+ version_added: ~
+ type: string
+ example: ~
+ default: "True"
+ fernet_key:
+ description: |
+ Secret key to save connection passwords in the db
+ version_added: ~
+ type: string
+ sensitive: true
+ example: ~
+ default: "{FERNET_KEY}"
+ donot_pickle:
+ description: |
+ Whether to disable pickling dags
+ version_added: ~
+ type: string
+ example: ~
+ default: "False"
+ default_impersonation:
+ description: |
+ If set, tasks without a ``run_as_user`` argument will be run with this
user
+ Can be used to de-elevate a sudo user running Airflow when executing
tasks
+ version_added: ~
+ type: string
+ example: ~
+ default: ""
+ unit_test_mode:
+ description: |
+ Turn unit test mode on (overwrites many configuration options with test
+ values at runtime)
+ version_added: ~
+ type: string
+ example: ~
+ default: "True"
+database:
+ description: ~
+ options:
+ sql_alchemy_conn:
+ description: |
+ The SqlAlchemy connection string to the metadata database.
+ SqlAlchemy supports many different database engines.
+ More information here:
+
http://airflow.apache.org/docs/apache-airflow/stable/howto/set-up-database.html#database-uri
+ version_added: 2.3.0
+ type: string
+ sensitive: true
+ example: ~
+ default: "sqlite:///{AIRFLOW_HOME}/airflow.db"
+ load_default_connections:
+ description: |
+ Whether to load the default connections that ship with Airflow. It's
good to
+ get started, but you probably want to set this to ``False`` in a
production
+ environment
+ version_added: 2.3.0
+ type: string
+ example: ~
+ default: "True"
+logging:
+ description: ~
+ options:
+ base_log_folder:
+ description: |
+ The folder where airflow should store its log files.
+ This path must be absolute.
+ There are a few existing configurations that assume this is set to the
default.
+ If you choose to override this you may need to update the
dag_processor_manager_log_location and
+ dag_processor_manager_log_location settings as well.
+ version_added: 2.0.0
+ type: string
+ example: ~
+ default: "{AIRFLOW_HOME}/logs"
+hive:
+ description: ~
+ options:
+ default_hive_mapred_queue:
+ description: |
+ Default mapreduce queue for HiveOperator tasks
+ version_added: ~
+ type: string
+ example: ~
+ default: "airflow"
+smtp:
+ description: |
+ If you want airflow to send emails on retries, failure, and you want to use
Review Comment:
```suggestion
If you want Airflow to send emails on retries, failure, and you want to
use
```
##########
airflow/config_templates/config_unit_tests.yml:
##########
@@ -0,0 +1,221 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Defaults specified in this file replace the default values from
+# config.yml file during unit tests. If you want a value to have
+# different default during unit tests, specify it here.
+
+---
+
+core:
+ description: ~
+ options:
+ dags_folder:
+ description: |
+ The folder where your airflow pipelines live, most likely a
Review Comment:
```suggestion
The folder where your Airflow pipelines live, most likely a
```
##########
airflow/config_templates/config_unit_tests.yml:
##########
@@ -0,0 +1,221 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Defaults specified in this file replace the default values from
+# config.yml file during unit tests. If you want a value to have
+# different default during unit tests, specify it here.
+
+---
+
+core:
+ description: ~
+ options:
+ dags_folder:
+ description: |
+ The folder where your airflow pipelines live, most likely a
+ subfolder in a code repository. This path must be absolute.
+ version_added: ~
+ type: string
+ example: ~
+ default: "{AIRFLOW_HOME}/dags"
+ executor:
+ description: |
+ The executor class that airflow should use. Choices include
+ ``SequentialExecutor``, ``LocalExecutor``, ``CeleryExecutor``,
``DaskExecutor``,
+ ``KubernetesExecutor``, ``CeleryKubernetesExecutor`` or the
+ full import path to the class when using a custom executor.
+ version_added: ~
+ type: string
+ example: ~
+ default: "LocalExecutor"
+ dags_are_paused_at_creation:
+ description: |
+ Are DAGs paused by default at creation
Review Comment:
Absolutely the smallest of nitpicks, some Description fields end with
punctuation, others don't.
--
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]
