dstandish commented on pull request #20578:
URL: https://github.com/apache/airflow/pull/20578#issuecomment-1004165936
@jedcunningham @BasPH after some time away from this one I think I see a
good way forward.
We can use the `kubernetes` config settings as a "default" for hook behavior.
I.e. we can update the hook so that, it optionally uses these config values
if not overrided by hook params or connection attrs.
```
if conf.getboolean('kubernetes', 'enable_tcp_keepalive'):
_enable_tcp_keepalive()
if not conf.getboolean('kubernetes', 'verify_ssl'):
_disable_verify_ssl()
```
If these two params are the only thing "missing" ( i.e. in the hook logic
compared with the get_kube_client logic ), then it should be fully backward
compatible. Then we can _always_ use the hook and the parameters (context,
config, in cluster) can be freely combined with the hook.
---
Do you have any concerns with using the `kubernetes` config settings as a
"default" for hook behavior? I suppose some could say this integrates provider
and core in a way that could potentially be undesirable. Though others might
think it is appropriate to have the K8s hook take the same defaults as K8s
executor.
If we're OK with that, then the question I just need to confirm is, is there
anything _else_ that differs between hook and the old get_kube_client
---
@jedcunningham one issue with deprecating use of those params is, maybe
people want to still use those params and not a conn id. Use of those params
only (i.e. and not also a conn_id) is supported with the hook. What I might
suggest we do though is add a dict param `hook_params` or `hook_kwargs` and
then deprecate those three params _as individual params_ in order to reduce
number of params in KPO and be more future proof.
--
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]
