instead of creating it in the VaultBackend constructor: when configured
(with host and port), SMTP.connect() is immediately called, which makes
it mandatory to have the smtp server up and running to be able to create
the VaultBackend object (which makes it hard to run properly in an elastic
environment like docker or k8s).
Details
Details
- Reviewers
vlorentz - Group Reviewers
Reviewers - Commits
- rDVAUcab17e72537b: Instanciate the SMTP class only when needed
Diff Detail
Diff Detail
- Repository
- rDVAU Software Heritage Vault
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
Build is green
Patch application report for D8205 (id=29604)
Rebasing onto ca71a59eda...
Current branch diff-target is up to date.
Changes applied before test
commit 18c958a42ba69823e93788e3c81e07b66311dbf5
Author: David Douard <david.douard@sdfa3.org>
Date: Fri Aug 5 19:16:21 2022 +0200
Instanciate the SMTP class only when needed
instead of creating it in the VaultBackend constructor: when configured
(with host and port), SMTP.connect() is immediately called, which makes
it mandatory to have the smtp server up and running to be able to create
the VaultBackend object (which makes it hard to run properly in an elastic
environment like docker or k8s).See https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/250/ for more details.
| swh/vault/backend.py | ||
|---|---|---|
| 492–502 | We can keep the previous behavior of leaving connections open. Why "this is doomed"? | |
| swh/vault/backend.py | ||
|---|---|---|
| 492–502 |
why would we want this? What are the pros of keeping it open?
because there is no reason to believe there is someone listening on localhost:25 (especially in elastic envs etc). | |
| swh/vault/backend.py | ||
|---|---|---|
| 492–502 |
less latency, but it's not a big deal
I'm fine with removing the fallback, I just didn't understand the comment. | |
Comment Actions
Build is green
Patch application report for D8205 (id=30529)
Rebasing onto ee678042f9...
Current branch diff-target is up to date.
Changes applied before test
commit cab17e72537b4188a2df4e2bedc6afa92d4f8810
Author: David Douard <david.douard@sdfa3.org>
Date: Fri Aug 5 19:16:21 2022 +0200
Instanciate the SMTP class only when needed
instead of creating it in the VaultBackend constructor: when configured
(with host and port), SMTP.connect() is immediately called, which makes
it mandatory to have the smtp server up and running to be able to create
the VaultBackend object (which makes it hard to run properly in an elastic
environment like docker or k8s).
This also removes the fallback to hardcoded 'localhost:25' smtp server;
if the smtp server is not configured or not reachable, the call to
_smtp_send() will fail logging the failure (both in logging and sentry).See https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/253/ for more details.