8000 [WIP][Greendns] Replace deprecated resolver.query by resolver.resolve by 4383 · Pull Request #840 · eventlet/eventlet · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[WIP][Greendns] Replace deprecated resolver.query by resolver.resolve #840

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[WIP][Greendns] Replace deprecated resolver.query by resolver.resolve #840

wants to merge 1 commit into from

Conversation

4383
Copy link
Member
@4383 4383 commented Dec 15, 2023

@codecov Codecov
Copy link
codecov bot commented Dec 15, 2023 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 23%. Comparing base (d78f8f6) to head (7243eeb).
Report is 86 commits behind head on master.

Files with missing lines Patch % Lines
eventlet/support/greendns.py 0% 4 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (d78f8f6) and HEAD (7243eeb). Click for more details.

HEAD has 12 uploads less than BASE
Flag BASE (d78f8f6) HEAD (7243eeb)
py39dnspython1 1 0
py39selects 1 0
py38selects 1 0
py38epolls 1 0
py39poll 1 0
py38openssl 1 0
py310selects 1 0
py310epolls 1 0
py310poll 1 0
py38poll 1 0
py39epolls 1 0
py311epolls 1 0
Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #840     +/-   ##
=======================================
- Coverage      53%    23%    -31%     
=======================================
  Files          88     88             
  Lines        9848   9983    +135     
  Branches     1841   1811     -30     
=======================================
- Hits         5299   2317   -2982     
- Misses       4164   7480   +3316     
+ Partials      385    186    -199
Flag Coverage Δ
ipv6 23% <0%> (+<1%) ⬆️
py310epolls ?
py310poll ?
py310selects ?
py311epolls ?
py38epolls ?
py38openssl ?
py38poll ?
py38selects ?
py39dnspython1 ?
py39epolls ?
py39poll ?
py39selects ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@4383 4383 mentioned this pull request Dec 15, 2023
Open
@4383 4383 force-pushed the dns_from_query_to_resolve branch from 759f76f to e5c5d2c Compare December 15, 2023 10:54
@4383 4383 changed the title [Greendns] Replace deprecated resolver.query by resolver.resolve [WIP][Greendns] Replace deprecated resolver.query by resolver.resolve Dec 15, 2023
@4383 4383 force-pushed the dns_from_query_to_resolve branch 2 times, most recently from 7cdedd0 to ec88ef2 Compare December 15, 2023 11:31
@jstasiak jstasiak mentioned this pull request Dec 15, 2023
Closed
@4383 4383 force-pushed the dns_from_query_to_resolve branch from ec88ef2 to a6a0f1b Compare December 15, 2023 11:43
@4383
Copy link
Member Author
4383 commented Dec 15, 2023 •
edited
Loading

This topic seems a bit more complicated than just replacing a deprecated method by a new one.
I followed the suggestions made in #818, but, local tests and CI jobs started to fail with assignation errors like this one:

FAILED tests/greendns_test.py::TestUdp::test_udp_ipv6 - OSError: [Errno 99] Cannot assign requested address

I need further investigation to see what happens.

The latest build is successful because I submitted a new patch set with an empty commit, just to compare...

@blink1073: Please can you give us more details on the way you made your tests and how you triggered this warning?

@4383
Copy link
Member Author
4383 commented Dec 15, 2023

Moving to the new function lead eventlet to:

>               raise NoNameservers(request=self.request, errors=self.errors)                                          
E               dns.resolver.NoNameservers: All nameservers failed to answer the query machine. IN A:

@4383 4383 force-pushed the dns_from_query_to_resolve branch from a6a0f1b to ad06bec Compare December 15, 2023 15:58
@blink1073
Copy link

Here's the full traceback from pymongo:

/opt/python/3.10/lib/python3.10/contextlib.py:135: in __enter__
 [2023/12/15 07:56:56.991]     return next(self.gen)
 [2023/12/15 07:56:56.991] pymongo/pool.py:1656: in checkout
 [2023/12/15 07:56:56.991]     conn = self._get_conn(handler=handler)
 [2023/12/15 07:56:56.991] pymongo/pool.py:1775: in _get_conn
 [2023/12/15 07:56:56.991]     conn = self.connect(handler=handler)
 [2023/12/15 07:56:56.991] pymongo/pool.py:1605: in connect
 [2023/12/15 07:56:56.991]     sock = _configured_socket(self.address, self.opts)
 [2023/12/15 07:56:56.991] pymongo/pool.py:1291: in _configured_socket
 [2023/12/15 07:56:56.991]     sock = _create_connection(address, options)
 [2023/12/15 07:56:56.991] pymongo/pool.py:1244: in _create_connection
 [2023/12/15 07:56:56.991]     for res in socket.getaddrinfo(host, port, family, socket.SOCK_STREAM):
 [2023/12/15 07:56:56.991] .tox/test-eg/lib/python3.10/site-packages/eventlet/support/greendns.py:549: in getaddrinfo
 [2023/12/15 07:56:56.991]     qname, addrs = _getaddrinfo_lookup(host, family, flags)
 [2023/12/15 07:56:56.991] .tox/test-eg/lib/python3.10/site-packages/eventlet/support/greendns.py:511: in _getaddrinfo_lookup
 [2023/12/15 07:56:56.991]     answer = resolve(host, qfamily, False, use_network=use_network)
 [2023/12/15 07:56:56.991] .tox/test-eg/lib/python3.10/site-packages/eventlet/support/greendns.py:456: in resolve
 [2023/12/15 07:56:56.991]     return _proxy.query(name, rdtype, raise_on_no_answer=raises,
 [2023/12/15 07:56:56.991] .tox/test-eg/lib/python3.10/site-packages/eventlet/support/greendns.py:412: in query
 [2023/12/15 07:56:56.991]     return end()
 [2023/12/15 07:56:56.991] .tox/test-eg/lib/python3.10/site-packages/eventlet/support/greendns.py:391: in end
 [2023/12/15 07:56:56.991]     raise result[1]
 [2023/12/15 07:56:56.991] .tox/test-eg/lib/python3.10/site-packages/eventlet/support/greendns.py:372: in step
 [2023/12/15 07:56:56.991]     a = fun(*args, **kwargs)
 [2023/12/15 07:56:56.991] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 [2023/12/15 07:56:56.991]     def query(
 [2023/12/15 07:56:56.991]         self,
 [2023/12/15 07:56:56.991]         qname: Union[dns.name.Name, str],
 [2023/12/15 07:56:56.991]         rdtype: Union[dns.rdatatype.RdataType, str] = dns.rdatatype.A,
 [2023/12/15 07:56:56.991]         rdclass: Union[dns.rdataclass.RdataClass, str] = dns.rdataclass.IN,
 [2023/12/15 07:56:56.991]         tcp: bool = False,
 [2023/12/15 07:56:56.991]         source: Optional[str] = None,
 [2023/12/15 07:56:56.991]         raise_on_no_answer: bool = True,
 [2023/12/15 07:56:56.991]         source_port: int = 0,
 [2023/12/15 07:56:56.991]         lifetime: Optional[float] = None,
 [2023/12/15 07:56:56.991]     ) -> Answer:  # pragma: no cover
 [2023/12/15 07:56:56.991]         """Query nameservers to find the answer to the question.
 [2023/12/15 07:56:56.991]         This method calls resolve() with ``search=True``, and is
 [2023/12/15 07:56:56.991]         provided for backwards compatibility with prior versions of
 [2023/12/15 07:56:56.991]         dnspython.  See the documentation for the resolve() method for
 [2023/12/15 07:56:56.991]         further details.
 [2023/12/15 07:56:56.991]         """
 [2023/12/15 07:56:56.991] >       warnings.warn(
 [2023/12/15 07:56:56.991]             "please use dns.resolver.Resolver.resolve() instead",
 [2023/12/15 07:56:56.991]             DeprecationWarning,
 [2023/12/15 07:56:56.991]             stacklevel=2,
 [2023/12/15 07:56:56.991]         )
 [2023/12/15 07:56:56.991] E       DeprecationWarning: please use dns.resolver.Resolver.resolve() instead

Our code that uses dnspython directly is here.

We test eventlet integration by applying this patch and then invoking pytest.main on our full test suite.

@4383 4383 force-pushed the dns_from_query_to_resolve branch from ad06bec to 7243eeb Compare December 18, 2023 09:30
@4383
Copy link
Member Author
4383 commented Dec 19, 2023

Thanks for the details. Unit tests should be fixed too, I need to rework my patch.

4383
4383 commented Dec 19, 2023
View reviewed changes
eventlet/support/greendns.py
@@ -400,7 +400,8 @@ def end():
return end()

# Main query
step(self._resolver.query, qname, rdtype, rdclass, tcp, source, raise_on_no_answer=False)
step(self._resolver.resolve, qname, rdtype, rdclass, tcp, source,
raise_on_no_answer=False, search=True)
Copy link
Member Author
@4383 4383 Dec 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

resolve and query doesn't have the same signature, and internally query call resolve with search=True, while this option is set to None by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
deprecation important
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use of deprecated dnspython method
2 participants
@4383 @blink1073
0