Migrate ext::auth::SMTPConfig -> cfg::EmailProvider
#7942
Conversation
scotttrinh
force-pushed
the
email-provider
branch
from
c740860 to
ed44ea0
Compare
|
Yeah, this is what I was thinking of basically. |
scotttrinh
force-pushed
the
email-provider
branch
4 times, most recently
from
02628d1 to
e34724d
Compare
| "EDGEDB_SERVER_AUTH_SMTP_CONCURRENCY", | ||
| os.environ.get("EDGEDB_SERVER_SMTP_CONCURRENCY", 5), |
There was a problem hiding this comment.
wdyt? I guess we need GEL stuff, too...
There was a problem hiding this comment.
Can this not go through the config system somehow?
There was a problem hiding this comment.
I guess "probably, but also the existing behavior doesn't"?
There was a problem hiding this comment.
Well, now is a great time to add it to the config!
scotttrinh
force-pushed
the
email-provider
branch
from
e8fdd90 to
c1195be
Compare
| }; | ||
|
|
||
| CREATE TYPE cfg::SMTPProviderConfig EXTENDING cfg::EmailProviderConfig { | ||
| CREATE PROPERTY sender -> std::str { |
There was a problem hiding this comment.
We should make sender required, I'm not sure why we didn't have that before.
| SET default := <std::duration>'15 seconds'; | ||
| CREATE ANNOTATION std::description := | ||
| "Maximum time for each SMTP request."; | ||
| }; |
There was a problem hiding this comment.
Max concurrency config here?
There was a problem hiding this comment.
Yeah, I think so. Though this would become a per-provider config, instead of a server/tenant-wide config. It should be fine for now, but in the long run we may want to have both: per-provider config makes sure of not overwhelming the provider, while the server/tenant-wide config makes sure e.g. the local number of FDs is not exceeded.
This will provide multiple choices for email providers allowing switching between providers.
Providers need to be set up per-instance to allow instance-wide fallback to a configured provider.
scotttrinh
force-pushed
the
email-provider
branch
from
7be7aff to
3121c31
Compare
This lets old SMTP config work when restoring dumps
Configure with something like:
```
EDGEDB_MAGIC_SMTP_CONFIG='[{"_tname": "cfg::SMTPProviderConfig", "validate_certs": true, "host": null, "password": null, "name": "cloud_dev_smtp", "port": null, "security": "STARTTLSOrPlainText", "username": null, "sender": "[email protected]", "timeout_per_email": "PT1M", "timeout_per_attempt": "PT15S"}]'
```
scotttrinh
force-pushed
the
email-provider
branch
from
ce126e6 to
a7303bc
Compare
| @@ -1282,6 +1283,20 @@ def get_default_tenant(self) -> edbtenant.Tenant: | |||
| def iter_tenants(self) -> Iterator[edbtenant.Tenant]: | |||
| yield self._tenant | |||
|
|
|||
| def _load_sidechannel_configs(self) -> None: | |||
| # TODO(fantix): Do something like this for multitenant | |||
|
Anything blocking this? |
This will provide multiple choices for email providers allowing switching
between providers. Moves the SMTP config into the main config namespace to allow for instance-wide configuration.