Skip to content
This repository has been archived by the owner on Sep 13, 2023. It is now read-only.

add bandit to pre-commit #424

Closed
wants to merge 8 commits into from
Closed

add bandit to pre-commit #424

wants to merge 8 commits into from

Conversation

vvssttkk
Copy link
Contributor

@vvssttkk vvssttkk commented Oct 3, 2022

close #61

also, bump some rev's

@vvssttkk vvssttkk requested a review from a team October 3, 2022 09:54
@vvssttkk vvssttkk temporarily deployed to external October 3, 2022 09:54 Inactive
@mike0sv
Copy link
Contributor

mike0sv commented Oct 3, 2022

Can you also fix bandit's warnings? We can probably ignore those in tests
Adopting #427 should fix other issues

@vvssttkk
Copy link
Contributor Author

vvssttkk commented Oct 3, 2022

will try, but bandit some time have false-positive errors/warnings

@vvssttkk vvssttkk temporarily deployed to external October 3, 2022 17:51 Inactive
@vvssttkk
Copy link
Contributor Author

vvssttkk commented Oct 3, 2022

some problem resolved; for few add #nosec because refactor shell is long and users should be understand that you can't do without it
another problems suggest you decide, because put usedforsecurity=False is not good

@vvssttkk
Copy link
Contributor Author

vvssttkk commented Oct 6, 2022

so, we have

put usedforsecurity=False not good, mb @mike0sv u know better solution?

@aguschin
Copy link
Contributor

Sorry @vvssttkk, Mike was on vacation last week. @mike0sv, could you please TAL at @vvssttkk's last question?

@mike0sv
Copy link
Contributor

mike0sv commented Oct 16, 2022

@vvssttkk we dont use it for security, just to fingerprint files, so I guess usedforsecurity=False is ok for our case

@vvssttkk vvssttkk temporarily deployed to external October 17, 2022 17:36 Inactive
@vvssttkk
Copy link
Contributor Author

@mike0sv @aguschin thx, updated code according last bandit warning

@vvssttkk vvssttkk temporarily deployed to external October 18, 2022 04:43 Inactive
@aguschin
Copy link
Contributor

@vvssttkk, thanks. I'm re-running the CI, but looks like there are some FAILED tests. Could you please TAL?

@vvssttkk vvssttkk temporarily deployed to external October 18, 2022 08:43 Inactive
@vvssttkk
Copy link
Contributor Author

hmmm.. ran local the tests/contrib/test_docker/test_context.py::test_dockerfile_generator_super_custom and did not get any error. they work normal, because made an intentional mistake and get the error

@aguschin aguschin changed the base branch from main to release/0.3.0 October 19, 2022 03:27
@aguschin aguschin temporarily deployed to external October 19, 2022 03:35 Inactive
@aguschin aguschin temporarily deployed to external October 19, 2022 05:47 Inactive
@aguschin aguschin changed the base branch from release/0.3.0 to main October 19, 2022 05:48
@aguschin
Copy link
Contributor

ok, was trying to merge with release/0.3.0 since this is a new upcoming release, but abandoned the idea since more issues emerged.

@aguschin
Copy link
Contributor

aguschin commented Oct 19, 2022

Seeing other failed tests now as well.
@vvssttkk, could I ask you to create another PR with your changes on top of release/0.3.0?

@vvssttkk vvssttkk closed this by deleting the head repository Oct 19, 2022
@aguschin aguschin added good first issue Good for newcomers and removed good first issue Good for newcomers labels Oct 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set up bandit
3 participants