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 [flake8].extra_files to allow configuring plugins like Bandit #16470

Merged
merged 4 commits into from Aug 15, 2022

Conversation

ShantanuKumar
Copy link
Contributor

Adds a new option for passing config files used byflake8 plugin.

Fixes #15225

[ci skip-rust]
[ci skip-build-wheels]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@Eric-Arellano Eric-Arellano changed the title add extra config files as an option to flake8 Add [flake8].extra_files to allow configuring plugins like Bandit Aug 10, 2022
Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Exciting!

src/python/pants/backend/python/lint/flake8/rules.py Outdated Show resolved Hide resolved
src/python/pants/backend/python/lint/flake8/subsystem.py Outdated Show resolved Hide resolved
@ShantanuKumar
Copy link
Contributor Author

I am trying to add a test but I think I am missing some understanding of how options etc are being passed in test

This is how the modified test_3rdparty_plugin looks like.

def test_3rdparty_plugin(rule_runner: RuleRunner) -> None:
    rule_runner.write_files(
        {"f.py": "'constant' and 'constant2'\n", "BUILD": "python_sources(name='t')"}
    )
    tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
    result = run_flake8(
        rule_runner,
        [tgt],
        extra_args=[
            "--flake8-extra-requirements=flake8-pantsbuild>=2.0,<3",
            "--flake8-lockfile=<none>",
        ],
    )
    assert len(result) == 1
    assert result[0].exit_code == 1
    assert "f.py:1:1: PB11" in result[0].stdout

    # Test extra_files option
    rule_runner.write_files(
        {
            "f.py": "assert None == 1\n",
            ".bandit": 'skips: ["B101"]\n',
            "BUILD": "python_sources(name='t')",
        }
    )
    tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
    result = run_flake8(
        rule_runner,
        [tgt],
        extra_args=[
            "--flake8-extra-requirements=flake8-bandit==3.0.0",
            "--flake8-lockfile=<none>",
            "--flake8-extra-files=[.bandit]",
        ],
    )
    assert len(result) == 1
    assert result[0].exit_code == 0

@ShantanuKumar
Copy link
Contributor Author

ShantanuKumar commented Aug 10, 2022

By the way how are these options propagated to the reference?
Is this automated?

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@Eric-Arellano
Copy link
Contributor

am trying to add a test but I think I am missing some understanding of how options etc are being passed in test

Hm what's the issue? Btw, I think it's fine to remove the flake8-pantsbuild part and only do the Bandit plugin. That is sufficient to prove that 3rd-party plugins can work.

By the way how are these options propagated to the reference?

A maintainer will run the generate_docs.py script to automatically update based on the help message you wrote :)

@ShantanuKumar
Copy link
Contributor Author

So it's just not this new test that I am trying to add which is failing. I think something else is wrong

./pants --changed-since=HEAD test
08:29:13.05 [ERROR] Completed: Run Pytest - src/python/pants/backend/python/lint/flake8/rules_integration_test.py:tests failed (exit code 2).
============================= test session starts ==============================
collected 0 items / 1 error

==================================== ERRORS ====================================
_ ERROR collecting src/python/pants/backend/python/lint/flake8/rules_integration_test.py _
ImportError while importing test module '/private/var/folders/9w/9_4n35r57d707zkrk86rvh9w0000gn/T/pants-sandbox-EgsQUA/src/python/pants/backend/python/lint/flake8/rules_integration_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/Users/developer/.pyenv/versions/3.7.9/lib/python3.7/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
src/python/pants/backend/python/lint/flake8/rules_integration_test.py:10: in <module>
    from pants.backend.python import target_types_rules
src/python/pants/backend/python/target_types_rules.py:19: in <module>
    from pants.backend.python.dependency_inference.module_mapper import (
src/python/pants/backend/python/dependency_inference/module_mapper.py:21: in <module>
    from pants.backend.python.subsystems.setup import PythonSetup
src/python/pants/backend/python/subsystems/setup.py:19: in <module>
    from pants.option.subsystem import Subsystem
src/python/pants/option/subsystem.py:12: in <module>
    from pants.engine.internals.selectors import AwaitableConstraints, Get
src/python/pants/engine/internals/selectors.py:24: in <module>
    from pants.engine.internals.native_engine import (
E   ImportError: dlopen(/private/var/folders/9w/9_4n35r57d707zkrk86rvh9w0000gn/T/pants-sandbox-EgsQUA/src/python/pants/engine/internals/native_engine.so, 0x0002): symbol not found in flat namespace (_PyCMethod_New)
- generated xml file: /private/var/folders/9w/9_4n35r57d707zkrk86rvh9w0000gn/T/pants-sandbox-EgsQUA/src.python.pants.backend.python.lint.flake8.rules_integration_test.py.tests.xml -
=========================== short test summary info ============================
ERROR src/python/pants/backend/python/lint/flake8/rules_integration_test.py
!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
=============================== 1 error in 0.52s ===============================

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@Eric-Arellano
Copy link
Contributor

Yeah, that's not related to options. That's that the native engine is not loading for you during test runs. Would you be open posting about this in #development in Slack? It will probably be easier to help you debug there.

(In the meantime, feel free to push the code to this PR and we can have CI see that the test works. Not ideal, but to unblock you.)

@Eric-Arellano Eric-Arellano enabled auto-merge (squash) August 11, 2022 15:55
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
auto-merge was automatically disabled August 11, 2022 18:23

Head branch was pushed to by a user without write access

@stuhood stuhood merged commit 065af0e into pantsbuild:main Aug 15, 2022
@ShantanuKumar ShantanuKumar deleted the flake8-extra-files branch August 16, 2022 07:32
cczona pushed a commit to cczona/pants that referenced this pull request Sep 1, 2022
…antsbuild#16470)

Adds a new option for passing config files used by`flake8` plugin.

Fixes pantsbuild#15225

[ci skip-rust]
[ci skip-build-wheels]
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.

Bandit plugin used by flake8 can't read the config from the bandit's config
3 participants