Switch HTTP client from hackney to finch #758
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
savhappy
commented
Jul 29, 2024
•
savhappy
commented
- switch from Hackney to Finch
- alter tests to pass with Finch
- closes Switch from Hackney to Finch as the default HTTP client #724
whatyouhide
reviewed
Jul 30, 2024
| @@ -253,30 +253,30 @@ defmodule Sentry.Config do | |||
| The maximum number of attempts to send an event to Sentry. | |||
| """ | |||
| ], | |||
| hackney_opts: [ | |||
There was a problem hiding this comment.
We need to keep hackney_opts and deprecate them if we want this to not be a breaking change. NimbleOptions supports deprecating options, check out the docs for that.
This also applies to the options below.
There was a problem hiding this comment.
Does this mean we need to keep the option:
client: [
type: :atom,
type_doc: "`t:module/0`",
default: Sentry.HackneyClient,
doc: """
"""
]and the module HackneyClient?? @whatyouhide
lib/sentry/finch_client.ex
Outdated
| HTTP client, you'll have to implement your own `Sentry.HTTPClient`. See the | ||
| documentation for `Sentry.HTTPClient` for more information. | ||
|
|
||
| Finch is built on top of NimblePool. If you need to set other pool configuration options, |
There was a problem hiding this comment.
Can you link to nimble_pool's repo here?
lib/sentry/finch_client.ex
Outdated
Comment on lines
15
to
16
| see "Pool Configuration Options" in the source code for details on the possible map values. | ||
| [finch configuration options](https://github.com/sneako/finch/blob/main/lib/finch.ex) |
There was a problem hiding this comment.
We shouldn't point people to source code, that can change (especially main). Let's link to Finch for docs
Comment on lines
+515
to
+516
| @spec finch_opts() :: keyword() | ||
| def finch_opts, do: fetch!(:finch_opts) |
There was a problem hiding this comment.
Where do we use these?
Co-authored-by: Andrea Leopardi <an.leopardi@gmail.com>
Co-authored-by: Andrea Leopardi <an.leopardi@gmail.com>
Co-authored-by: Andrea Leopardi <an.leopardi@gmail.com>
Co-authored-by: Andrea Leopardi <an.leopardi@gmail.com>
savhappy
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.