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

Properly initialize external modules #2328

Merged
merged 4 commits into from
Feb 7, 2022

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Feb 2, 2022

What was wrong?

We weren't properly initializing external modules that do not inherit from the web3.module.Module class. This led to class methods not working as there was no self to reference.

Also, based on our docs, we expected to be able to initialize an external module with passing in a Web3 instance as the first argument. This was not the case and wasn't tested.

How was it fixed?

  • Properly initialize modules that do not inherit from the Module class by adding ()
  • Allow for modules to accept a Web3 instance as the first argument of their __init__() if it requires use of the web3 instance

Todo:

  • Add entry to the release notes
  • Add cute animal picture
  • Update docs

Cute Animal Picture

GOPR4249

@fselmo fselmo force-pushed the bugfix-external-modules branch 2 times, most recently from d944553 to 5e33862 Compare February 2, 2022 16:43
@wolovim wolovim marked this pull request as ready for review February 2, 2022 18:55
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.

LGTM!

docs/web3.main.rst Outdated Show resolved Hide resolved
web3/_utils/module.py Outdated Show resolved Hide resolved
fselmo and others added 2 commits February 4, 2022 13:01
Properly initialize modules that do not inherit from the `web3.module.Module` class. We weren't properly testing self-referential, non-static methods with this functionality and so a test was also added for this.
@fselmo fselmo force-pushed the bugfix-external-modules branch 2 times, most recently from 27713d7 to c0c5dac Compare February 7, 2022 18:21
* Allow for accepting the ``Web3`` instance as the first argument in any module's ``__init()`` method, rather than requiring a module to inherit from ``web3.module.Module`` if it needs to make use of the ``Web3`` instance.

* Update tests to test the above change.

* Add a more friendly error message if the module has more than one __init__() argument. Add test for this error message / case.
@fselmo fselmo merged commit e4331ea into ethereum:master Feb 7, 2022
@fselmo fselmo deleted the bugfix-external-modules branch February 7, 2022 19:19
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.

3 participants