scottlimmer opened a new pull request, #43413:
URL: https://github.com/apache/airflow/pull/43413
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #34554
related: #41539
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
closes: #34554
related: #41539
**WIP** This change is incomplete, requires discussion.
Mandatory configuration of `smtp_user` and `smtp_pass` via `Connection`
object creates confusion as to source of truth for other SMTP params. This
change proproses overriding all `smtp_*` vars with the `Connection` object
equivalents when `email.email_conn_id` is specified. However, this creates some
a questions that need resolving/consensus.
## Issue 1
My preference would be to use the `[smtp]` config vars as default/fallback
values. However, this isn't possible as the connection UI doesn't allow for an
'not-specified' state for certain fields.
E.g. the 'Disable SSL' checkbox defaults to `False` when not set, making it
impossible to create a fallback condition. Attempting to implement a fallback
leads to a state where the `smtp_ssl` config is ignored if the checkbox is
checked, or the `smtp_ssl` config must be set to the desired value if the
checkbox is unchecked.
## Issue 2
`[smtp]` configuration variables are become usable only for connections not
requiring authentication. This effectively reverses the confusion I'm trying to
address. I.e. `[smtp]` config vars would have no effect if `email_conn_id` is
in play.
# Possible solutions
* Remove all `[smtp]` config vars duplicated by connection object.
Effectively mandate that all smtp configuration is done via a `Connection`
object. This requires the most changes.
* Split `airflow.utils.email.send_email_smtp` backend into
`send_email_smtp` and `smtp_email_smtp_via_conn`. For this to work we'd need to
revert #41539 and its deprecation, then auto-select backend based on presence
of `email_conn_id`. Effectively states that if you want to use a `Connection`
object to secure your SMTP params, you must configure the entirety of the SMTP
connection using the object.
* Introduct another config: `smtp.smtp_config_source=config|connection` and
require the user to opt into desired behaviour. Allows the user to specify that
all SMTP vars should be controlled by `Connection` object. Still leaves a
confusing default state.
* Modify the the connection UI to allow the user to clarify intent, thereby
enabling the use of smtp config as fallback/default.
* Change checkboxes to Default/Yes/No
* Provide No auth/user/pass checkbox to user to specify smtp doesn't
require login.
* Other ideas?
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)**
for more information.
In case of fundamental code changes, an Airflow Improvement Proposal
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals))
is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party
License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a
newsfragment file, named `{pr_number}.significant.rst` or
`{issue_number}.significant.rst`, in
[newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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]
