potiuk commented on code in PR #33452:
URL: https://github.com/apache/airflow/pull/33452#discussion_r1298033831
##########
airflow/configuration.py:
##########
@@ -1307,6 +1307,16 @@ def has_option(self, section: str, option: str) -> bool:
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 = str(section).lower()
+ option = str(option).lower()
+ super().set(section, option, value)
Review Comment:
There is literally one place were set is used in airflow outside of tests
(where we use conf_vars context manager), and there is hardly a case where
somoene would like to use it programmatically - mostly becauset there is a lot
happening when airflow gets initialized and conf is initialized very early and
then by the time you could do "set", it's already effective and something else
likely used it already, so `set` was not really something that we would expect
people to use.
But they could have done some kind of hacks in tasks - for example -
nevertheless. But I agree that setting somethign and NOT using it would seem
like a bug on ther side, and while it might break their workflow, they should
fix their code.
So yes - agree it's likely something of a bugfix type, however it does call
for a "significant" newsfragment in case someone gets some error and will want
to check in the reflog if something has changed vs. previous version of
Airflow.
--
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]
