Skip to content

Add network marks to tests that fail without an internet connection#8881

Merged
jrbourbeau merged 2 commits intodask:mainfrom
phobson:test-network-marks
Apr 6, 2022
Merged

Add network marks to tests that fail without an internet connection#8881
jrbourbeau merged 2 commits intodask:mainfrom
phobson:test-network-marks

Conversation

@phobson
Copy link
Copy Markdown
Contributor

@phobson phobson commented Apr 5, 2022

  • Closes #xxxx
  • Tests added / passed
  • Passes pre-commit run --all-files

Flying over to DC, I had the occasion to the run the test suite without the internet and think that a few more tests could benefit from the @pytest.mark.network decorator.

@GPUtester
Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

@phobson phobson changed the title TST: add network marks to tests that failed on the plane add network marks to tests that failed on the plane Apr 5, 2022
@jrbourbeau
Copy link
Copy Markdown
Member

add to allowlist

Copy link
Copy Markdown
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks @phobson. This seems reasonable to me. I'm surprised dask/tests/test_distributed.py::test_map_partitions_df_input requires an internet connection as it appears to only involve local computations. I'll test this out locally in a bit (unless you have a traceback handy)

@phobson
Copy link
Copy Markdown
Contributor Author

phobson commented Apr 6, 2022

@jrbourbeau I meant to comment on that on here (and probably should in the code too).

I also found it surprising. In airplane mode, I got an http error trying to hit 8.8.8.8 (IIRC). I can revisit that real fast. I wouldn't be surprised if that test is an abuse of the network mark.

await c.compute(df.map_partitions(f, arr, meta=df._meta))


@pytest.mark.network
Copy link
Copy Markdown
Contributor Author

@phobson phobson Apr 6, 2022

Choose a reason for hiding this comment

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

@jrbourbeau

This one is a stretch. With all network connections killed, the error message I'm getting without the decorator is in the details below. It's not actually an error -- just a failure due to a raised warning.

Details
>       with distributed.LocalCluster(
            scheduler_port=0,
            dashboard_address=":0",
            asynchronous=False,
            n_workers=1,
            nthreads=1,
            processes=False,
        ) as cluster:

dask/tests/test_distributed.py:634:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/deploy/local.py:236: in __init__
    super().__init__(
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/deploy/spec.py:260: in __init__
    self.sync(self._start)
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/utils.py:309: in sync
    return sync(
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/utils.py:376: in sync
    raise exc.with_traceback(tb)
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/utils.py:349: in f
    result = yield future
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/tornado/gen.py:762: in run
    value = future.result()
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/deploy/spec.py:293: in _start
    self.scheduler = await self.scheduler
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/core.py:299: in _
    await self.start()
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/scheduler.py:4180: in start
    await self.listen(
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/core.py:437: in listen
    listener = await listen(
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/comm/core.py:366: in listen
    return backend.get_listener(loc, handle_comm, deserialize, **kwargs)
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/comm/inproc.py:343: in get_listener
    return InProcListener(loc, handle_comm, deserialize)
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/comm/inproc.py:250: in __init__
    self.address = address or self.manager.new_address()
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/comm/inproc.py:63: in new_address
    return "%s/%d/%s" % (self.ip, os.getpid(), next(self.addr_suffixes))
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/comm/inproc.py:39: in ip
    self._ip = get_ip()
../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/utils.py:155: in get_ip
    return _get_ip(host, port, family=socket.AF_INET)
cytoolz/functoolz.pyx:476: in cytoolz.functoolz._memoize.__call__
    ???
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

host = '8.8.8.8', port = 80, family = <AddressFamily.AF_INET: 2>

    @toolz.memoize
    def _get_ip(host, port, family):
        # By using a UDP socket, we don't actually try to connect but
        # simply select the local address through which *host* is reachable.
        sock = socket.socket(family, socket.SOCK_DGRAM)
        try:
            sock.connect((host, port))
            ip = sock.getsockname()[0]
            return ip
        except OSError as e:
>           warnings.warn(
                "Couldn't detect a suitable IP address for "
                "reaching %r, defaulting to hostname: %s" % (host, e),
                RuntimeWarning,
            )
E           RuntimeWarning: Couldn't detect a suitable IP address for reaching '8.8.8.8', defaulting to hostname: [Errno 51] Network is unreachable

../../mambaforge/envs/dask-mypy/lib/python3.9/site-packages/distributed/utils.py:135: RuntimeWarning

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, yeah, that is what's happening. I'd just silence that warning in setup.cfg by adding it to filterwarnings rather than doing anything specific to this test. That warning is innocuous in our tests.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1 for not elevating this specific warning to an error. @phobson, as @jcrist mentioned, adding a section like

# With no network access, distributed raises a warning when detecting local IP address
ignore:Couldn't detect a suitable IP address:RuntimeWarning:distributed

here

dask/setup.cfg

Lines 46 to 54 in 0b36d7f

filterwarnings =
# From Cython-1753
ignore:can't resolve:ImportWarning
error:::dask[.*]
error:::pandas[.*]
error:::numpy[.*]
error:::distributed[.*]
# From https://github.com/numpy/numpy/issues/20225
ignore:The distutils\.sysconfig module is deprecated, use sysconfig instead:DeprecationWarning:numpy

will allow us to not raise an unnecessary error for these cases

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks @jcrist & @jrbourbeau. I modified the setup.cfg per the suggestion and tests passed locally w/o a network. Just pushed a commit with that change and reverting the change to test_distributed.py

@jcrist
Copy link
Copy Markdown
Member

jcrist commented Apr 6, 2022

IIRC distributed sometimes tries to open a network connection to 8.8.8.8 determine the local ip, that might be what you're seeing?

@jrbourbeau jrbourbeau changed the title add network marks to tests that failed on the plane Add network marks to tests that fail without an internet connection Apr 6, 2022
@jrbourbeau jrbourbeau mentioned this pull request Apr 6, 2022
Copy link
Copy Markdown
Member

@jrbourbeau jrbourbeau left a comment

Choose a reason for hiding this comment

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

Thanks @phobson! Will merge after CI finishes

Note the gpuCI failures are unrelated (xref #8825)

@jrbourbeau jrbourbeau merged commit 9bdc32a into dask:main Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants