jlaportebot opened a new pull request, #66490:
URL: https://github.com/apache/airflow/pull/66490
Fixes #66391
## Problem
The `test_connection` method in the SMTP hook was incorrectly checking if
the return value from `smtp_client.noop()` was equal to 250. However, according
to Python's `smtplib` documentation, `noop()` returns a tuple `(status_code,
message)`, not just an integer.
This caused the connection test to always fail even when the connection was
successful, because the tuple `(250, b'2.0.0 Ok')` would never equal the
integer `250`.
## Solution
Changed the check from:
```python
if status == 250:
```
to:
```python
if status[0] == 250:
```
This correctly accesses the first element of the tuple (the status code) to
check if it equals 250.
## Changes
- **File**: `providers/smtp/src/airflow/providers/smtp/hooks/smtp.py`
- Line 304: Changed `if status == 250:` to `if status[0] == 250:`
- **File**: `providers/smtp/tests/unit/smtp/hooks/test_smtp.py`
- Added `test_test_connection_success`: Verifies successful connection
test when noop returns (250, message)
- Added `test_test_connection_failure_non_250`: Verifies failure when noop
returns non-250 status
- Added `test_test_connection_exception`: Verifies exception handling
## Testing
Added comprehensive tests to verify the fix:
- Test that `test_connection` returns success when `noop()` returns `(250,
b'2.0.0 Ok')`
- Test that `test_connection` returns failure when `noop()` returns non-250
status
- Test that `test_connection` handles exceptions gracefully
All tests verify that the method correctly handles the tuple return value
from `smtplib.noop()`.
## Impact
This fix ensures that the SMTP connection test in the Airflow UI works
correctly, allowing users to verify their SMTP configuration is working
properly.
--
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]
