Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: wrong configuration for the eth connection pool. #117

Merged
merged 4 commits into from
Aug 28, 2024

Conversation

Hartick
Copy link
Contributor

@Hartick Hartick commented Aug 27, 2024

Context

The example listener application doesn't start due to a must provide a positive size error.

Why it doesn't work?

The must provide a positive size error is raised by the github.com/hashicorp/golang-lru/v2 package which is used by client/eth/connection_pool.go while creating the LRU caches.

The LRU cache requires a number of client URLs greater than 0, but no one was passed because of a misconfiguration in the TOML file: Config.ConnectionPool is not under the App sub-struct.

Solution

  • The App prefix has been removed from examples/listener/config.toml
  • Added checks on the length of the client's URLs set within the Config struct in the NewConnectionPoolImpl function.

* add a specific error for missing RPC URL

* fix key prefix in config.toml of the example listener
@Hartick Hartick requested a review from calbera August 27, 2024 20:52
@Hartick Hartick added the bug Something isn't working label Aug 27, 2024
@calbera calbera requested a review from hunter-bera August 27, 2024 20:53
@calbera
Copy link
Contributor

calbera commented Aug 27, 2024

@Hartick can run make format && make lint from the project root to fix the linter

@calbera calbera closed this Aug 27, 2024
@calbera calbera reopened this Aug 27, 2024
* make WS client an optional one

* add tests for connection_pool.go

* fix lint errors
* rename function getClient in getClientFrom

* move mutex usage to user-facing functions
@calbera
Copy link
Contributor

calbera commented Aug 28, 2024

@Hartick Would be good to add the nil check when .Add is called on the caches. Fix tests and then LGTM

* Add a checkEnv function used to skip the test if the env vars are not set.

* Remove unneeded nil checks for HTTP clients cache

* Check if the WS Url cache is null before adding WS clients to the cache.
@calbera calbera added this pull request to the merge queue Aug 28, 2024
Merged via the queue into main with commit 5b4dab8 Aug 28, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants