Adding connectivity check and warning warning #43
Conversation
staking_deposit/deposit.py
Outdated
| Checks if there is an internet connection and warns the user if so. | ||
| ''' | ||
| try: | ||
| urlopen('https://www.google.com/', timeout=2) |
There was a problem hiding this comment.
I'm not a fan of this test here. It potentially tests too much (DNS, TCP, HTTPS, TLS and maybe a few other). For any quick replies, it would give a strong confidence that an Internet connection is present, but there is a non-significant chance of timeout and false negative.
I think I would prefer something like this DNS resolution:
import socket # up in the import section
socket.setdefaulttimeout(2)
socket.getaddrinfo('icann.org', 80)
icann.org being a domain name that is less likely to be blocked in Russia, China, Iran, Turkmenistan, Libya, and Sudan among others.
A DNS test is not perfect, but it would fail faster on a non-connected device. It would also reduce the false negatives.
There was a problem hiding this comment.
See https://docs.python.org/3/library/socket.html for the socket module documentation.
There was a problem hiding this comment.
I missed the socket callout in the issue notes.
Updated.
|
I think |
staking_deposit/deposit.py
Outdated
| socket.setdefaulttimeout(2) | ||
| socket.getaddrinfo('icann.org', 80) | ||
| click.pause(load_text(['connectivity_warning'])) | ||
| except: # noqa: E722 |
There was a problem hiding this comment.
This should only catch OSError since all socket exceptions are subclasses of OSError and that should enable us to catch anything raised by socket calls here. It will also enable us to remove the noqa: E722.
Updating required python version to 3.9 Adding test_deposit.py
valefar-on-discord
force-pushed
the
connectivity-warning
branch
from
64eeaa7 to
4080a18
Compare
|
Updated to ignore connectivity check if
|
Adding a check when running the CLI to see if the user has an active internet connection and warn them if so.
Also adding a flag
--ignore-connectivitywhich will not run the check.I tried to add a test with the CLIRunner and ran into a bit of an issue where the tests have no problem waiting for inputs through
click.prompt, but when usingclick.pausewhich allows for any keypress, it appears the tests ignore this.I didn't want to go over the top on using prompt and having the user acknowledge in a specific manner. I feel a keypress is fine.
Fixes #17