This is an automated email from the ASF dual-hosted git repository.
taragolis pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new abbd5677ba make `conf.set` case insensitive (#33452)
abbd5677ba is described below
commit abbd5677bab4a84b1d35e7723c7dfbb155ca9144
Author: Raphaël Vandon <[email protected]>
AuthorDate: Mon Aug 21 14:14:33 2023 -0700
make `conf.set` case insensitive (#33452)
* make `conf.set` case insensitive
`conf.get` is insensitive (it converts section and key to lower case)
but set is not, which can lead to surprising behavior
(see the test, which is not passing without the fix).
I suggest that we override set as well to fix that.
Any value that was set before with upper case was unreacheable.
* fix remove_option as well
* away with the str()
* add significant change newsfragment
---
airflow/configuration.py | 16 ++++++++++++++--
newsfragments/33452.significant.rst | 11 +++++++++++
tests/core/test_configuration.py | 7 +++++++
3 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/airflow/configuration.py b/airflow/configuration.py
index 3c9c6975ea..41dc8d718e 100644
--- a/airflow/configuration.py
+++ b/airflow/configuration.py
@@ -949,8 +949,8 @@ class AirflowConfigParser(ConfigParser):
_extra_stacklevel: int = 0,
**kwargs,
) -> str | None:
- section = str(section).lower()
- key = str(key).lower()
+ section = section.lower()
+ key = key.lower()
warning_emitted = False
deprecated_section: str | None
deprecated_key: str | None
@@ -1306,6 +1306,16 @@ class AirflowConfigParser(ConfigParser):
except (NoOptionError, NoSectionError):
return False
+ def set(self, section: str, option: str, value: str | None = None) -> None:
+ """
+ Set an option to the given value.
+
+ This override just makes sure the section and option are lower case,
to match what we do in `get`.
+ """
+ section = section.lower()
+ option = option.lower()
+ super().set(section, option, value)
+
def remove_option(self, section: str, option: str, remove_default: bool =
True):
"""
Remove an option if it exists in config from a file or default config.
@@ -1313,6 +1323,8 @@ class AirflowConfigParser(ConfigParser):
If both of config have the same option, this removes the option
in both configs unless remove_default=False.
"""
+ section = section.lower()
+ option = option.lower()
if super().has_option(section, option):
super().remove_option(section, option)
diff --git a/newsfragments/33452.significant.rst
b/newsfragments/33452.significant.rst
new file mode 100644
index 0000000000..59a5d485ba
--- /dev/null
+++ b/newsfragments/33452.significant.rst
@@ -0,0 +1,11 @@
+``conf.set()`` becomes case insensitive to match ``conf.get()`` behavior.
Also, ``conf.get()`` will now break if used with non-string parameters.
+
+``conf.set(section, key, value)`` used to be case sensitive, i.e.
``conf.set("SECTION", "KEY", value)``
+and ``conf.set("section", "key", value)`` were stored as two distinct
configurations.
+This was inconsistent with the behavior of ``conf.get(section, key)``, which
was always converting the section and key to lower case.
+
+As a result, configuration options set with upper case characters in the
section or key were unreachable.
+That's why we are now converting section and key to lower case in ``conf.set``
too.
+
+We also changed a bit the behavior of ``conf.get()``. It used to allow objects
that are not strings in the section or key.
+Doing this will now result in an exception. For instance,
``conf.get("section", 123)`` needs to be replaced with ``conf.get("section",
"123")``.
diff --git a/tests/core/test_configuration.py b/tests/core/test_configuration.py
index 110aba182d..a7caba2f70 100644
--- a/tests/core/test_configuration.py
+++ b/tests/core/test_configuration.py
@@ -111,6 +111,13 @@ class TestConf:
assert conf.get("core", "PERCENT") == "with%inside"
assert conf.get("CORE", "PERCENT") == "with%inside"
+ @conf_vars({("core", "key"): "test_value"})
+ def test_set_and_get_with_upper_case(self):
+ # both get and set should be case insensitive
+ assert conf.get("Core", "Key") == "test_value"
+ conf.set("Core", "Key", "new_test_value")
+ assert conf.get("Core", "Key") == "new_test_value"
+
def test_config_as_dict(self):
"""Test that getting config as dict works even if
environment has non-legal env vars"""
