Add Snake Case Methods(Shh) #1433
Conversation
|
Is this Correct @pipermerriam? |
There was a problem hiding this comment.
This is a great start @Patil2099!
In addition to my comments in the code, there are a few more things that need to be done:
- Add the methods to eth-tester: https://github.com/ethereum/web3.py/pull/1326/files#diff-a9220b36fd339b0c77ccc1158707aa0aR268
- Add the new methods to the
GethShhmodule inweb3/geth.py - Make sure that the old methods are raising a
DeprecationWarningin the tests. See here for an example.
Thanks!
|
@kclowes @pipermerriam I made more changes Please Review Them. |
There was a problem hiding this comment.
Looking good @Patil2099! Based on the failing integration tests, it looks like a few of the methods didn't get moved to snake case. The missing methods that I see are has_key_pair, has_sym_key, generate_sym_key_from_password, and set_max_message_size.
If you could also check that the correct deprecation warnings are raised in the tests, that would be great. Example can be found here. Thanks @Patil2099!
|
@kclowes I made half changes to the test are those correct? I wanted to know if it is correct then I will proceed with it also can you give me an example if I am wrong |
|
Okay I get it. I will make a PR in some time
On Aug 29, 2019 10:22 PM, kclowes <[email protected]> wrote:
@kclowes commented on this pull request.
________________________________
In tests/core/shh-module/test_shh_key_pair.py<#1433 (comment)>:
assert web3.shh.has_key_pair(key_id)
assert len(web3.shh.get_public_key(key_id)) == 132
-
- private_key = web3.shh.get_private_key(key_id)
+ with pytest.warns(DeprecationWarning):
+ private_key = web3.shh.get_private_key(key_id)
This new snake case method shouldn't raise the DeprecationWarning, the old camel case methods should. It would be good to keep the old tests for the camel case methods to make sure they're still working until they're removed, we just want to make sure that every time we use the old methods, we issue a DeprecationWarning. You should be able to copy/paste this test and make another one like:
def test_shh_asymmetric_key_pair(web3, skip_if_testrpc):
...
with pytest.warns(DeprecationWarning):
key_id = web3.shh.newKeyPair()
assert web3.shh.hasKeyPair(key_id)
assert len(web3.shh.getPublicKey(key_id)) == 132
...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1433?email_source=notifications&email_token=AIQAR5D4OOIOM2I3MY6C7UDQG75EJA5CNFSM4IPJOP5KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCDEL4NI#pullrequestreview-281591349>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AIQAR5GHC5RD3BQDA3IVFWLQG75EJANCNFSM4IPJOP5A>.
|
|
@kclowes @pipermerriam I made some more changes. Please review them and let me know if some other change is required. |
|
@kclowes @pipermerriam I made all the changes that were required. Please Review My PR |
|
@Patil2099 All the tests should be passing. Once those are all green, I'll take a closer look! |
|
@kclowes Can you please help a little bit? |
| from .base import ( # noqa: F401 | ||
| BaseProvider, | ||
| JSONBaseProvider, | ||
| ) | ||
|
|
||
| from .rpc import HTTPProvider # noqa: F401 |
There was a problem hiding this comment.
@Patil2099 I believe your tests are erroring right now because these lines are order-dependent.
|
@Patil2099 I made some updates, and screwed up your history in the process. Sorry about that. I'll pick it back up on Monday. |
|
@Patil2099 I got the tests running, and made some updates to the tests to get them passing. I also squashed all the commits down to one. I'm happy to expand on either of those things more if you are interested in picking up another module. Thanks for all of your work! |
#1429