uranusjr commented on a change in pull request #20184:
URL: https://github.com/apache/airflow/pull/20184#discussion_r766235448
##########
File path: airflow/utils/helpers.py
##########
@@ -244,3 +244,18 @@ def build_airflow_url_with_query(query: Dict[str, Any]) ->
str:
view = conf.get('webserver', 'dag_default_view').lower()
url = url_for(f"Airflow.{view}")
return f"{url}?{parse.urlencode(query)}"
+
+
+def exactly_one(*args):
+ """
+ Returns True if exactly one of *args is "truthy", and False otherwise.
+ :param args:
+ :return:
+ """
+ has_one = False
+ for arg in args:
+ if arg and has_one:
+ return False
+ elif arg:
+ has_one = True
+ return has_one
Review comment:
> I went with `exactly_one(execution_date, run_id)` because AFAIK those
values obey truthy evaluation. Are you suggesting that we do `is not None`
because we should never do truthy evaluation? Or because in this particular
case there is a reason not to?
Because `run_id` can be an empty string, and we want to show a different
error for this situation.
> can you clarify?
Basically the above. The main purpose of this function (well, the xor checks
it is replacing) is to check whether arguments are passed, not whether those
arguments are falsy. And if you don’t either explicitly use `is not None`, or
include stricter evaluation logic in `exactly_one`, this creates a subtle
semantic difference between the caller site and the body of `exactly_one` that
can potentially trip people up and cause subtle, unintended behavioural change.
And this situation is complete avoidable by changing the evaluation logic to
not rely on falsiness.
--
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]
