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

[disk] handle multilines df output #2733

Merged
merged 1 commit into from Aug 9, 2016
Merged

Conversation

degemer
Copy link
Member

@degemer degemer commented Aug 4, 2016

What does this PR do?

remove missing method _is_number in disk check.

Motivation

Fix #2732

Additional Notes

The previous method was:

try:
    float(my_var)
except:
    return False
return True

@degemer degemer added this to the 5.8.6 milestone Aug 4, 2016
@degemer
Copy link
Member Author

degemer commented Aug 4, 2016

This is not the right way to do it. I'll update once we know more.

@degemer
Copy link
Member Author

degemer commented Aug 4, 2016

Updated with a test, it works now!

@degemer degemer changed the title [disk] remove missing method _is_number [disk] handle multilines df output Aug 4, 2016
tmpfs tmpfs 32969616 0 32969616 0% /dev/shm
/dev/sda1 ext4 512752 90460 395412 19% /boot
10.1.5.223:/vil/cor
nfs 1020054752 56080768 963973984 6% /cor
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: the output seems to be aligned by column, it might be good to make sure the fixture has all the extra white spaces this implies (see

linux_df_k = """Filesystem 1K-blocks Used Available Use% Mounted on
/dev/sda1 8256952 5600592 2236932 72% /
none 3802316 124 3802192 1% /dev
none 3943856 0 3943856 0% /dev/shm
none 3943856 148 3943708 1% /var/run
none 3943856 0 3943856 0% /var/lock
none 3943856 0 3943856 0% /lib/init/rw
/dev/sdb 433455904 305360 411132240 1% /mnt
/dev/sdf 52403200 40909112 11494088 79% /data
nfs:/abc/def/ghi/jkl/mno/pqr
52403200 40909112 11494088 79% /data
/dev/sdg 52403200 40909112 11494088 79% /data
"""
)

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice 👀

@olivielpeau
Copy link
Member

Added 2 nitpicks, but overall this looks 💚 !

Feel free to merge once they're addressed

When moving the disk check outside of `checks/`, `_is_number` method was
forgotten.
This is now fixed and a test is added.
@degemer
Copy link
Member Author

degemer commented Aug 8, 2016

Thanks for the review, I addressed both comments, waiting for 🍏 .

@olivielpeau
Copy link
Member

💚 (CI failure is caused by an elasticsearch test that fails during setup, unrelated)

@degemer degemer merged commit 6b07d09 into master Aug 9, 2016
@degemer degemer deleted the quentin/disk-is_number branch August 9, 2016 15:38
@truthbk truthbk modified the milestones: 5.9.0, 5.8.6 Aug 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants