Skip to content

Remove distutils dependency from python 3 version of the check#14245

Merged
alopezz merged 2 commits into
masterfrom
alopez/network-remove-distutils
Mar 27, 2023
Merged

Remove distutils dependency from python 3 version of the check#14245
alopezz merged 2 commits into
masterfrom
alopez/network-remove-distutils

Conversation

@alopezz

@alopezz alopezz commented Mar 27, 2023

Copy link
Copy Markdown
Contributor

What does this PR do?

Uses shutil.which in place of distutils.spawn.find_executable for Python 3 to remove dependency on distutils

Motivation

AI-2974. We want to get rid of distutils because its version in the stdlib is essentially deprecated, and the replacement used by setuptools increases the agent's memory footprint. This is the only place in core integrations that uses distutils directly.

Additional Notes

The reason that distutils is still used for Python 2 is that shutil.which was added on Python 3.3. There are less concerns about the use of distutils in Python 2, so missing a simple stand-in, it's fine to keep the statu quo there.

I haven't found a good way to add an integration test for this, so I've done a simple manual sanity check on the e2e environment to ensure this is doing what we expect:

root@docker-desktop:/# /opt/datadog-agent/embedded/bin/python
  Python 3.8.16 (default, Mar 27 2023, 11:47:24)
  [GCC 5.4.0 20160609] on linux
  Type "help", "copyright", "credits" or "license" for more information.
  >>> from datadog_checks.network.network import find_executable
  >>> find_executable('ss')
  '/usr/bin/ss'

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached
  • If the PR doesn't need to be tested during QA, please add a qa/skip-qa label.

@alopezz
alopezz requested review from a team as code owners March 27, 2023 13:30
@ghost ghost added the integration/network label Mar 27, 2023
@codecov

codecov Bot commented Mar 27, 2023

Copy link
Copy Markdown

Codecov Report

Merging #14245 (f307fb3) into master (e3de73a) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Flag Coverage Δ
network 94.01% <100.00%> (+0.96%) ⬆️

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

@iliakur iliakur left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the comment explaining why all the song and dance!

@alopezz
alopezz merged commit 4a6ee57 into master Mar 27, 2023
@alopezz
alopezz deleted the alopez/network-remove-distutils branch March 27, 2023 15:29
steveny91 pushed a commit that referenced this pull request Mar 30, 2023
)

* Use shutil.which instead of distutils.spawn.find_executable on Python3

This frees us from depending on distutils on Python 3

* Remove out of date sentence from docstring
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.

2 participants