-
Notifications
You must be signed in to change notification settings - Fork 242
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
Added py3dns
recipe
#728
Added py3dns
recipe
#728
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's wait for the CI to do its jobs, but overall looks good.
I'm only worried about forcing users to use 8.8.8.8
without warning them.
Did you checked for alternate solutions? (E.g. Like exposing an API via the ios
module?)
If we decide that using the Google DNS is the only feasible and easy way to do it, how about including ´8.8.4.4´ as a fallback?
I dont now how to use ios module for that, but using 8.8.4.4 is a good solution (and easy). For exmaple we try to check 8.8.8.8 using socket, if it does not be used, we switch to 8.8.4.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI noticed an issue on PEP8 checks, can you take care of it?
yes, I make new commit with some patch fixes |
@misl6 How about this check script? import socket
def check_host(host="8.8.8.8", port=53, timeout=3):
"""
Host: 8.8.8.8 (google-public-dns-a.google.com)
OpenPort: 53/tcp
Service: domain (DNS/TCP)
"""
try:
socket.setdefaulttimeout(timeout)
socket.socket(socket.AF_INET, socket.SOCK_STREAM).connect((host, port))
return True
except socket.error as ex:
return False
host = "8.8.8.8"
if not check_host(host):
host = "8.8.4.4" |
Isn't |
Of course, I dont think about it :) |
@misl6 Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ouch. Looks like the patch contains some extra un-needed lines.
Can you take care of it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The automated tests on changed recipes are now failing due to a malformed patch.
2022-07-10T08:59:48.6004950Z STDERR:
2022-07-10T08:59:48.6005440Z /usr/bin/patch: **** malformed patch at line 13: @@ -50,8 +51,12 @@ defaults= { 'protocol':'udp', 'port':53, 'opcode':Opcode.QUERY,
Can you test it locally and check the patch?
I update patch file. cd py3dns-3.2.1
git init
git add .
git commit -m "test"
# make changes in file
git diff > ios.patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Same as in the android, but I change localhost to 8.8.8.8