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

Improved IP address fetch & display #87

Merged
merged 1 commit into from
Oct 30, 2020
Merged

Improved IP address fetch & display #87

merged 1 commit into from
Oct 30, 2020

Conversation

shivangswain
Copy link
Collaborator

Added a simple & better method to fetch and display public IP address in the server verbose

Closes #23

@shivangswain shivangswain self-assigned this Oct 29, 2020
Copy link
Owner

@gridhead gridhead left a comment

Choose a reason for hiding this comment

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

You would want to make the lines L#117 and L#121, the part of the ipaddress function so as to avoid repeating the same parts of code. Also, please make use of a try-except block in the function to address the cases when the server providing those addresses is unreachable.

(Don't worry about the specificity of the exception. Just catch one and let the user know that the addresses could not be fetched.)

Comment on lines +23 to +30
def ipaddress(v):
url = "https://api64.ipify.org"
if v == 4:
url = "https://api.ipify.org"
elif v == 6:
url = "https://api6.ipify.org"
response = urllib3.PoolManager().request('GET', url)
return response.data.decode('UTF-8')
Copy link
Owner

Choose a reason for hiding this comment

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

Nice work done with fetching distinct addresses.

@@ -1,4 +1,4 @@
import asyncio, websockets, sys, click, time, os
import asyncio, websockets, sys, click, urllib3, time
Copy link
Owner

Choose a reason for hiding this comment

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

urllib3 is a not-so elegant module but lightweight - something which is perfect for our usecase. Thanks for replacing requests in favour of this module.

Copy link
Owner

@gridhead gridhead left a comment

Choose a reason for hiding this comment

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

Just make changes listed on #87 (review).

@shivangswain
Copy link
Collaborator Author

Just pushed a new, even simpler and faster implementation

@shivangswain shivangswain merged commit 2851290 into gridhead:master Oct 30, 2020
shivangswain pushed a commit that referenced this pull request Oct 30, 2020
This reverts commit 2851290, reversing
changes made to 3be467b.
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.

Fetch and display the IPv4/IPv6 address on the verbose
2 participants