refactor nettools and add tests #531
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Using socket.create_connection instead of socket.connect This allow the function to work for both IPv4 and IPv6 addresses vs the current current implementation that support only IPv4 (AF_INET). also if host is a non-numeric hostname, it will try to resolve it for both AF_INET and AF_INET6, and then try to connect to all possible addresses in turn until a connection succeeds.
- Respecting the timeout arg passed to nettools.wait_http_test() function - Add a customized interval_time with a default value 2 - update the Docstring
- adding arg to allow fake the user-agent, default to True. - use standard library, avoid unnecessary dependencies and make the code more explicit. - add proper errors handlers. - update the function docstring.
- add compatibility for IPv6. - update the docstring to be more clearer. - add a default value for port arg as it does not matter much to simplifying the function call.
- Add compatibility to IPv6 in nettools.get_default_ip_config() - parameterizing the hard coded value for the ip address used to get the puplic facing ip
- make use of Json instead of depending or regex parsing. - optimize the code to get only the device that matter when pass 'device' arg to the function, instead of looping over all device. - depending on one shell command instead of multiple commands. - handle the error in cases when OS not supported. - update docstring. - now return cidr for both ipv4 and ipv6. 'ip', 'ip6' dict keys conatin list of (ip, prefix) pairs. - update of other nettools functions that use old version output. get_default_ip_config() and _netobject_get()
- getting more acurate timeout by passing the timeout value to ping command instead of implementing it with a while loop - handle the errors when running on unsupported os
Codecov Report
@@ Coverage Diff @@
## development #531 +/- ##
===============================================
+ Coverage 68.31% 70.68% +2.36%
===============================================
Files 91 91
Lines 4561 4608 +47
===============================================
+ Hits 3116 3257 +141
+ Misses 1445 1351 -94
Continue to review full report at Codecov.
|
xmonader
reviewed
Jan 3, 2021
xmonader
reviewed
Jan 3, 2021
…test - add ipv6 compatibilty - message default to empty bytes object instead of the make no sense b'PING' message - expecting to receive at least one byte response from the server as indicator of connection success instead of old value 8192 - add proper docstring
waleedhammam
approved these changes
Jan 20, 2021
this is necessary to fix repeated test docs #553
AhmedHanafy725
approved these changes
Jan 25, 2021
abom
approved these changes
Feb 4, 2021
abom
suggested changes
Feb 4, 2021
could we leave it for now? it will be useful in this stage in debugging any connection issues, later we can make it less and benefit from the performance gain. what do you think? |
abom
changed the title
abom
approved these changes
Mar 7, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Resolves #526
Changes
all commits: https://github.com/threefoldtech/js-ng/pull/531/commits
Known issues:
NotImplementedErroron other platforms.Related Issues
Resolves #526
Resolves #569
related to #427
Checklist