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

fix: refactor from ConfigFileFinder to load_config #35

Merged
merged 4 commits into from
Aug 19, 2022

Conversation

lsorber
Copy link
Contributor

@lsorber lsorber commented Aug 1, 2022

Attempt at fixing #33 by refactoring from flake8<5.0.0's ConfigFileFinder to flake8>=5.0.0,<6.0.0's load_config.

@calumy
Copy link

calumy commented Aug 6, 2022

This change now requires a .bandit file to be added; otherwise, this error is raised Unable to parse config file: The specified config file does not exist: .bandit. The Bandit docs suggest that the .bandit file is optional.

Is this change intentional, or is there a way that this can be avoided?

If this file is required, it might be worth updating the README.md to reflect this.

@lsorber
Copy link
Contributor Author

lsorber commented Aug 6, 2022

@calumy Apologies for the misleading output on stderr. A .bandit config file was and still is optional after this PR.

The message you're referring to was originally intended for when the [bandit] section of the .bandit file is missing, but this PR would also (erroneously) cause that message to be printed if the .bandit file itself is missing. This is entirely harmless. I've just pushed a commit that removes the message. Now, if the .bandit file is missing or not correctly formatted, its contents (if any) will silently be ignored.

@calumy
Copy link

calumy commented Aug 6, 2022

@lsorber Thanks for sorting this!

@calumy
Copy link

calumy commented Aug 6, 2022

Another possible way of sorting this would be to update the if statement to ignore the file not found error along with the unable to parse config file error. This would still allow an error when the [bandit] section of the .bandit file is missing as was originally intended but ignore the error if the .bandit file is missing altogether.

if str(e) != "No section: 'bandit'" and str(e) != "The specified config file does not exist: .bandit":
    sys.stderr.write(f"Unable to parse config file: {e}")

@lsorber
Copy link
Contributor Author

lsorber commented Aug 7, 2022

@calumy Sure, that could be done, but OTOH I'm not sure it's this plugin's responsibility to verify the validity of the .bandit file. For example, the [bandit] section could exist but one of its settings may be misspelled. I'll leave it up to @tylerwince to decide on what to do with this.

setup.py Outdated
@@ -22,7 +22,7 @@ def get_version(fname="flake8_bandit.py"):
VERSION = get_version()

# What packages are required for this module to be executed?
REQUIRED = ["flake8", "bandit>=1.7.3", "flake8-polyfill", "pycodestyle"]
REQUIRED = ["flake8>=5.0.0", "bandit>=1.7.3", "flake8-polyfill", "pycodestyle"]
Copy link

Choose a reason for hiding this comment

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

FWIW neither flake8-polyfill nor pycodestyle are used anywhere in this repo; maybe we could remove the dependency at the same time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@davfsa
Copy link

davfsa commented Aug 11, 2022

@tylerwince would it be possible to merge this and make a new release?

@tylerwince tylerwince merged commit 0e37cde into tylerwince:master Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants