Skip to content
This repository has been archived by the owner on Jul 22, 2024. It is now read-only.

add bandit #33

Merged
merged 15 commits into from
Jun 1, 2020
Merged

add bandit #33

merged 15 commits into from
Jun 1, 2020

Conversation

bdwyer2
Copy link

@bdwyer2 bdwyer2 commented May 26, 2020

  • add bandit security checks
  • update code to comply with bandit
  • update tests to expect the correct error type
  • switch from flask-restplus to flask-restx

@bdwyer2 bdwyer2 requested review from xuhdev and djalova May 26, 2020 21:58
maxfw/utils/image_functions.py Outdated Show resolved Hide resolved
maxfw/utils/image_functions.py Outdated Show resolved Hide resolved
assert sum(encoding[:-1]) == 0, \
'A Standardize or Normalize transformation must be positioned at the end of the pipeline.'
if sum(encoding[:-1]) != 0:
raise ValueError('A Standardize or Normalize transformation must be positioned at the end of the pipeline.')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
raise ValueError('A Standardize or Normalize transformation must be positioned at the end of the pipeline.')
raise ValueError('A Standardize or Normalize transformation can only be positioned at the end of the pipeline.')

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe another if-clause for encoding[-1]?

Copy link
Author

Choose a reason for hiding this comment

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

@xuhdev something like this:
if sum(encoding[:-1]) != 0 or encoding[-1]:?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but the error message would be hard to write if done in one if-clause

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't look like the same thing to me, maybe @djalova can confirm?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to ask @kmh4321

Copy link
Author

Choose a reason for hiding this comment

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

@xuhdev maybe for now I can revert 3e404c2 since that change is somewhat unrelated to this PR and open an issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure

Copy link
Author

Choose a reason for hiding this comment

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

CC: #34

bdwyer2 and others added 9 commits May 26, 2020 15:39
Co-authored-by: Hong Xu <hong@topbug.net>
Co-authored-by: Brendan Dwyer <brendan.dwyer@ibm.com>
Co-authored-by: Brendan Dwyer <brendan.dwyer@ibm.com>
Copy link
Contributor

@xuhdev xuhdev left a comment

Choose a reason for hiding this comment

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

LGTM

@bdwyer2 bdwyer2 merged commit 97b82ea into IBM:master Jun 1, 2020
@bdwyer2 bdwyer2 deleted the bandit branch June 2, 2020 00:00
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.

None yet

3 participants