dstandish commented on issue #15016:
URL: https://github.com/apache/airflow/issues/15016#issuecomment-837110088
> Why is clean_bool being called in the first place for a user-provided
dictionary? I'm not sure how this is necessary because the user can provide a
literal boolean value in the dictionary if needed, no? If in the event that a
driver needs to take a case-sensitive boolean string for some parameter, then
clean_bool would make it impossible to provide such a value.
TLDR I agree we should remove `clean_bool`
As to why it was put in, in the first place, here's what I think happened.
Observe what happens with airflow's connection URI format when we try to
pass a boolean value:
```python
>>> c = Connection(conn_id='hello', uri='hello://?my_val=True')
>>> c.extra_dejson
{'my_val': 'True'}
```
It's impossible to produce a json object with boolean values.
So when you are using top level key-value pairs in conn `extra` then in some
cases it makes sense to cast to bool.
I suspect maybe initially in the development of this hook the connect kwargs
were top level within `extra`, where doing this cast would make sense.
But when dealing with nested json, the boolean vs. string issue becomes
irrelevant and you have new problems to solve. Namely, at the time this hook
was merged, airflow's conn URI did not support nested json. So this hook did
not actually allow for storage of `connect_kwargs` in extra when using the URI
format. For that, you'd have to add a json.loads-if-str conversion
[here](https://github.com/apache/airflow/blob/2bee173ef71ab67a288a5513df37914e3dd4a6ce/airflow/providers/odbc/hooks/odbc.py#L165).
But now that we have [support for arbitrary json in conn
extra](https://github.com/apache/airflow/pull/15100), there's no need for such
a conversion.
So since `connect_kwargs` is nested json, there's no valid reason for
converting to bool, and I suspect this was just an oversight, and accordingly,
even though it could be fixed, it is best to remove.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
