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

Make flake8-bandit work with latest bandit 1.7.3 too #23

Merged
merged 2 commits into from
Mar 8, 2022

Conversation

sathieu
Copy link
Contributor

@sathieu sathieu commented Feb 28, 2022

Fixes: #21

flake8-bandit 1.7.3 (PyCQA/bandit#496) introduced an fdata argument and this just passes a None to make things work with the latest version of bandit.

Fixes: tylerwince#21

flake8-bandit 1.7.3 (PyCQA/bandit#496)
introduced an `fdata` argument and this just passes a `None` to make
things work with the latest version of bandit.
@sathieu
Copy link
Contributor Author

sathieu commented Feb 28, 2022

@tylerwince Please review.

@WilliamDEdwards
Copy link

Can we do anything to help get this merged, @tylerwince?

metrics=Metrics(),
)
except TypeError:
# bandit < 1.7.3 (https://github.com/tylerwince/flake8-bandit/issues/21)

Choose a reason for hiding this comment

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

Personally I would use the other PR approach, no real need to create this compatibility code. It just makes the code less maintainable and not a huge gain...

Choose a reason for hiding this comment

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

I'm not sure if we should pin versions in a package such as this one ...

Choose a reason for hiding this comment

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

I agree @staticdev , the TypeError catch here makes an assumption that BanditNodeVisitor won't change again in future. We may just end up with a new generic TypeError being caused by the older call below, when in fact the root cause would be something else entirely.

Better to make two updates in this package, I feel:

@tylerwince
Copy link
Owner

Hey @sathieu and @nastra, I'm just getting to look at these PRs. Give me a day or so to review. I need to compare this PR and the other one opened.

@tylerwince tylerwince merged commit 49e8dcc into tylerwince:master Mar 8, 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.

Bandit 1.7.3 addition of new positional argument fdata causes TypeError
5 participants