Replace Finch for Req as default swoosh client in installer #5882
Conversation
Swoosh has supported Req since 1.14.1. Although Req uses Finch under the hood, Req is a more "batteries included" approach. This means that there is no need to start an HTTP client manually, as was required by Finch.
| @@ -75,7 +75,7 @@ config :<%= @web_app_name %>, <%= @endpoint_module %>, | |||
| # domain: System.get_env("MAILGUN_DOMAIN") | |||
| # | |||
| # For this example you need include a HTTP client required by Swoosh API client. | |||
| # Swoosh supports Hackney and Finch out of the box: | |||
| # Swoosh supports Hackney, Req and Finch out of the box: | |||
There was a problem hiding this comment.
Perhaps we should just remove this section from the docs? We already config a client for prod anyway?
There was a problem hiding this comment.
Agreed, keep the part about configuring the adopter, drop the part about the HTTP Client.
| {:swoosh, "~> 1.5"}, | ||
| {:finch, "~> 0.13"},<% end %> | ||
| {:swoosh, "~> 1.16"}, | ||
| {:req, "~> 0.5.4"},<% end %> |
There was a problem hiding this comment.
We can probably relax it a bit?
| {:req, "~> 0.5.4"},<% end %> | |
| {:req, "~> 0.5"},<% end %> |
There was a problem hiding this comment.
I went for 0.5.x because I thought 0.6 may introduce breaking changes. Happy to go with either though.
There was a problem hiding this comment.
As you prefer! It is only for new apps and there will be a lock file anyway. And we can always update it.
| {:swoosh, "~> 1.5"}, | ||
| {:finch, "~> 0.13"}<% end %> | ||
| {:swoosh, "~> 1.16"}, | ||
| {:req, "~> 0.5.4"}<% end %> |
There was a problem hiding this comment.
| {:req, "~> 0.5.4"}<% end %> | |
| {:req, "~> 0.5"}<% end %> |
| {:swoosh, "~> 1.5"}, | ||
| {:finch, "~> 0.13"},<% end %> | ||
| {:swoosh, "~> 1.16"}, | ||
| {:req, "~> 0.5.4"},<% end %> |
There was a problem hiding this comment.
Just so you know, I'm considering some changes before Req v1.0 that are maybe worth mentioning here:
-
Since day 1, Req retries on errors by default. I've had some feedback over the years this is a bit surprising. There's a recently opened issue too: Retrying by default is a bit surprising wojtekmach/req#383. If you'd like to chime in, that's appreciated. I added some extra context in Swoosh too: https://github.com/swoosh/swoosh/pull/830/files#r1694016381.
-
Since day 1, Req brings in jason dependency. I'd like to drop it before v1.0 in favour of built-in :json from OTP 27. I'd be OK even conditionally vendoring it in for pre-27. This is tracked here:
json: Support optional use of:jsonin OTP 27+ wojtekmach/req#386.
Btw, Finch is dropping :castore dependency and once Finch is out with this change, I'm keen on requiring that Finch version as minimum in Req. I'd probably cut Req v0.6.0 for this change. This is effectively requiring OTP 25 (and running on system that have built-in CA) for everything to work out of the box.
{:req, "~> 0.5.4"} is definitely good for now. I'd be happy to send a patch bumping that to ~> 0.6/1.0 in the future.
Dunno how relevant are these for Phoenix but I thought Id mentioned it anyway!
There was a problem hiding this comment.
Both Swoosh and Phoenix depends on Jason, so making it optional on Req won't affect us. I also don't think OTP 25 requirement is an issue, this is for new projects and that's the minimum supported OTP version nowadays anyway.
There was a problem hiding this comment.
Regarding req retries by default and swoosh, this is all moot, sorry about the noise. In req the default is retry: :safe_transient, safe meaning only for GET/HEAD requests. Sending emails would be typically done using POST. So we are good.
Swoosh has supported Req since 1.14.1. Although Req uses Finch under the hood, Req is a more "batteries included" approach. This means that there is no need to start an HTTP client manually, as was required by Finch.