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

Change percentage bars to use an explicit exception for value checks #683

Closed

Conversation

kevinlondon
Copy link
Contributor

The percentage bars will now use an AssertionError directly if the provided values are outside of the expected range (0-100 inclusive).

I was using Bandit to check for potential vulnerabilities in a few popular Python projects and this came up as a warning:

>> Issue: Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
    Severity: Low   Confidence: High
    Location: glances/glances/outputs/glances_bars.py:72
 70      @percent.setter
 71      def percent(self, value):
 72          assert value >= 0
 73          assert value <= 100
 74          self.__percent = value

As per the warning, assert statements are stripped out if compiling with optimized bytecode (-O option) in Python. Here's a StackOverflow question with a little discussion and the original Python docs.

Oh, as a quick postscript, these tests run fine on OSX, so perhaps the Linux checks could be made to include OSX when running the tests?

@asergi
Copy link
Collaborator

asergi commented Sep 20, 2015

Hi @kevinlondon,

Thanks for your contribution. I manually merged the PR into develop with commit 2b1b80c.

I guess those assert statements were added for debugging purposes and then left there.
About the tests, I will have a look later.

Again, thanks for your contribution.

@asergi asergi closed this Sep 20, 2015
asergi added a commit that referenced this pull request Sep 21, 2015
@asergi
Copy link
Collaborator

asergi commented Sep 21, 2015

Tests unblocked: 837193e.

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.

None yet

2 participants