SamWheating commented on PR #33680: URL: https://github.com/apache/airflow/pull/33680#issuecomment-1692444567
> My motivation was that it is far more likely that when you want to inject a connection to K8s pod it would be one already defined in Airflow connections and being used by other operators rather than one stored elsewhere and you want to just bake it into the task. Also if that was not the case all you need is just to define a new connection. Basicly anything Airflow knows will also be accesable from K8s pod. Agreed - the same is true of this implementation. > I think it's also easier to explain to users - just tell us what is the conn_id you want and we will make sure it's available in the pod. When referencing it just decode the attributes. I think that this actually adds some complexity for users, as they have to understand two largely different mechanisms for mounting secrets. I also feel like it removes a lot of configurability with the existing secrets, such as mounting in env vs. a volume, which keys to mount etc. Since we already have all of this in place in this operator (with `secrets=[]`), why not re-use it? > it takes a list of (connection_id, env_var_prefix) as argument, and it creates a single secret in runtime and mounts the secret using deploy_type=env, for the secrets names, I use <env_var_prefix>_LOGIN, <env_var_prefix>_PASSWORD I have similar thoughts about this approach - it introduces more hidden functionality and requires users to be familiar with the implementation details and how they differ from the existing `secrets: list[Secret]` implementation. -- 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]
