Skip to content

Conversation

@kclowes
Copy link
Collaborator

@kclowes kclowes commented Aug 28, 2024

What was wrong?

@antazoey requested that get_request_headers be exposed as a class method.

Closes #3466

How was it fixed?

Added a check to see where to get the class name from, and a combomethod decorator, so it can be called either way.

Todo:

  • Clean up commit history
  • Add or update documentation related to these changes
  • Add entry to the release notes

Cute Animal Picture

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

@kclowes kclowes marked this pull request as ready for review August 28, 2024 22:08
@kclowes kclowes requested review from fselmo, pacrob and reedsa August 28, 2024 22:08
@kclowes kclowes force-pushed the get-request-headers-combo branch from 2615579 to 6e2410e Compare August 29, 2024 15:48
Copy link
Collaborator

@fselmo fselmo 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
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.

🐑

@kclowes kclowes merged commit 86827d6 into ethereum:main Aug 29, 2024
@kclowes kclowes deleted the get-request-headers-combo branch August 29, 2024 16:29
@souliane
Copy link

souliane commented Dec 7, 2024

Hi, I am wondering why you chose to use @combomethod instead of the stdlib @classmethod? It would also work with self.get_request_headers():

In [1]: class A:
   ...:     @classmethod
   ...:     def test(cls):
   ...:         print(cls)
   ...: 

In [2]: A.test()
<class '__main__.A'>

In [3]: A().test()
<class '__main__.A'>

Did I miss something?

@kclowes
Copy link
Collaborator Author

kclowes commented Dec 9, 2024

Mostly codebase consistency, and to make it explicit that calling it either way is fine and by design.

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.

Make HTTPProvider.get_request_headers() a class-level method

4 participants