Skip to content
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

Add validation to catch legacy imports #6081

Merged
merged 7 commits into from
Mar 19, 2020
Merged

Conversation

mgarabed
Copy link
Member

Motivation

The datadog_checks package was rearranged for all packages to be hosted under the base sub package. The original module locations have been updated to redirect to those imports for backwards compatibility, but usage of them is deprecated.

This tool will identify those imports and report them as errors, for future use in our CI.

It also includes an autofix option, to aid in the initial mass migration of changing all the files en masse before we introduce this validation to CI.

@mgarabed mgarabed requested a review from a team as a code owner March 17, 2020 22:29
@ofek ofek changed the title Add support for import validation Add validation to catch legacy imports Mar 18, 2020
@codecov
Copy link

codecov bot commented Mar 18, 2020

Codecov Report

Merging #6081 into master will decrease coverage by 7.17%.
The diff coverage is n/a.

Impacted Files Coverage Δ
datadog_checks_dev/tests/test_docker.py 48.14% <0%> (-51.86%) ⬇️
datadog_checks_dev/tests/tooling/test_utils.py 68.75% <0%> (-31.25%) ⬇️
datadog_checks_dev/datadog_checks/dev/env.py 22.72% <0%> (-20.46%) ⬇️
datadog_checks_dev/datadog_checks/dev/docker.py 24.57% <0%> (-19.5%) ⬇️
...atadog_checks_dev/datadog_checks/dev/structures.py 68.85% <0%> (-18.04%) ⬇️
datadog_checks_dev/tests/test_conditions.py 82.25% <0%> (-17.75%) ⬇️
datadog_checks_dev/datadog_checks/dev/_env.py 54.79% <0%> (-9.59%) ⬇️
...atadog_checks_dev/datadog_checks/dev/subprocess.py 72.97% <0%> (-5.41%) ⬇️
...atadog_checks_dev/datadog_checks/dev/conditions.py 80.95% <0%> (-4.77%) ⬇️
...ts/tooling/configuration/consumers/test_example.py 100% <0%> (ø) ⬆️
... and 885 more

echo_debug(f'Checking {check_name}')

# focus on check and testing directories
bases = [os.path.join(integrations_root, check_name, base) for base in ('datadog_checks', 'tests')]
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: some of the file/directory logic could be moved to functions in utils

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you clarify this one a bit? Which functions would be useful in as a utility function?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking something like turning this line into get_python_files_for_check(check) or something like that. It could be useful for future scripts

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a good idea - but may be better suited for a separate PR. There are several options which could be helpful and not sure those discussions would be suited for here.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@ChristineTChen ChristineTChen left a comment

Choose a reason for hiding this comment

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

Nice! Tested out locally

Validating imports avoiding deprecated modules ...

Validation failed: 152 deprecated imports found in 113 files:

  /Users/christine.chen/dev/integrations-core/active_directory/datadog_checks/active_directory/active_directory.py: line # 3
    from datadog_checks.checks.win import PDHBaseCheck

  /Users/christine.chen/dev/integrations-core/activemq_xml/datadog_checks/activemq_xml/activemq_xml.py: line # 8
    from datadog_checks.checks import AgentCheck
...

@mgarabed mgarabed merged commit 26d7834 into master Mar 19, 2020
@mgarabed mgarabed deleted the mgarabed/import-validation branch March 19, 2020 20:37
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