Configure Finch pools#169
Merged
akoutmos merged 6 commits intoakoutmos:masterfrom Sep 25, 2022
Merged
Conversation
patmaddox
commented
Sep 15, 2022
| - Git SHA of the last commit (if the GIT_SHA environment variable is present) | ||
| - Git author of the last commit (if the GIT_AUTHOR environment variable is present) | ||
|
|
||
| * `:finch_pools` - (optional) A map that will be passed to Finch.start_link/1 as the :pools key, |
Contributor
Author
There was a problem hiding this comment.
I opted for finch_pools rather than finch: [ pools: ... ] for two reasons:
Finch.start_link/1only defines two options:nameandpools. If they add more options at some point, this could get messy - but if that happened, we could consider changing this to justfinch.- More importantly,
PromEx.grafana_client_child_spec/4passes:nametoGrafanaClient.child_spec/, which passes it to Finch.
This would lead to some ambiguity if you did e.g. finch: [ name: MyApp, pools: %{ ... }].
It's possible to address - maybe just set a default name if the Finch options key doesn't exist. I just don't know that it's worth it at this point.
finch: [ pools: ... ] is more future-proof though, and I think would be straightforward to do.
Any preference on which it should be?
Owner
There was a problem hiding this comment.
I think the way that you have it now it fine given that finch doesn't provide any other configuration options that need to be leveraged. Will merge in as soon as CI passes :)
This file contains hidden or 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
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.
Change description
This adds a
finch_poolskey to the Grafana config, to be passed as thepoolskey toFinch.start_link/1.What problem does this solve?
We connect to our Grafana instance over an HTTP proxy. Configuring a proxy in Finch requires passing connection opts to the underlying Mint library.
Example usage
Additional details and screenshots
This bumps the Finch dependency, because v0.12.0 adds "support for
Mint.UnsafeProxyconnections". I opted to bump to the latest 0.13.0.Checklist