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

Retrieve endpoint_uri or ipc_path from base persistent provider class #3319

Merged

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Mar 29, 2024

What was wrong?

Since we defined endpoint_uri at the PersistentConnectionProvider level, we used it in the manager.py since it wouldn't raise any attribute errors. Rather than rename the ipc_path in AsyncIPCProvider, I think it makes sense to have this separation of paths but be able to retrieve either path from the base class as well since we may want to implement logic for the base class (as is the case in manager.py).

How was it fixed?

  • Implement endpoint_uri_or_ipc_path property in the base class to be able to return the proper path depending on the type of persistent connection provider.

Todo:

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

fselmo added a commit to fselmo/web3.py that referenced this pull request Mar 29, 2024
@fselmo fselmo requested review from pacrob and reedsa March 29, 2024 22:20
@fselmo fselmo force-pushed the redefine-persistent-connection-provider-paths branch from 0c7596b to 7596de6 Compare March 29, 2024 22:23
@fselmo fselmo marked this pull request as ready for review March 29, 2024 22:23
Copy link
Contributor

@pacrob pacrob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I think this is a good solution. Edit - Nit: maybe call the method get_endpoint_uri_or_ipc_path? Feel free to take or leave

web3/providers/persistent/persistent.py Show resolved Hide resolved
@fselmo fselmo merged commit a70618b into ethereum:main Apr 2, 2024
81 checks passed
@fselmo fselmo deleted the redefine-persistent-connection-provider-paths branch April 2, 2024 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants