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.

Sign up for GitHub

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

Added some utility in disk library #3637

Closed
wants to merge 1 commit into from
Closed

Added some utility in disk library #3637

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions avocado/utils/disk.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,17 @@
import os
import json
import re
import string

from . import process


class DiskUtilsError(Exception):
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an excellent intention, but I suggest you use a more specific exception, like OSError, for example.

Copy link
Member

Choose a reason for hiding this comment

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

OSError is generic, too.

Copy link
Contributor

Choose a reason for hiding this comment

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

@beraldoleal, I agree, but it, at least, covers a more specific set of exceptions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@willianrampazzo idea here is it catch Exceptions for utility, right now it seems OSError but idea is that Exceptions can be reused for any exception from this utility as I am planning to add more library will use this, please let me your thought

Copy link
Member

Choose a reason for hiding this comment

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

IMO, DiskUtilsError is better than an OSError, because tells the user that the error was in our library. But, I might be missing something.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@beraldoleal I also agree with this @willianrampazzo please let us know your thought so we can have V2

Copy link
Contributor

Choose a reason for hiding this comment

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

My initial comment was about changing class DiskUtilsError(Exception) by class DiskUtilsError(OSError) as OSError is more specific to what is been covered here, and not changing the DiskUtilsError by OSError.

But I don't have problems keeping the way it is now, it was just a suggestion to restrict the kind of exceptions covered by DiskUtilsError.

Copy link
Collaborator Author

@PraveenPenguin PraveenPenguin Mar 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@willianrampazzo sure but I feel we need to have DiskUtilsError having broader Exception so we need not to change it on future but when we raise it in utility we can narrow down there like in OSError case we have to expect in that utility how that sound

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, I got it. Let's go with Exception.

"""
Base Exception Class for all DiskUtils Error
"""


def freespace(path):
fs_stats = os.statvfs(path)
return fs_stats.f_bsize * fs_stats.f_bavail
Expand Down Expand Up @@ -87,3 +94,41 @@ def get_filesystem_type(mount_point='/'):
_, fs_file, fs_vfstype, _, _, _ = mount_line.split()
if fs_file == mount_point:
return fs_vfstype


def get_io_scheduler_list(device_name):
"""
Returns io scheduler available for the IO Device
:param device_name: Device name example like sda , hda
:return: list of IO scheduler
"""
names = open(__sched_path(device_name)).read()
Copy link
Contributor

Choose a reason for hiding this comment

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

You are using context manager all over the code, so just for the sake of consistency, my suggestion is to use a context manager here too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure make sense

return names.translate(string.maketrans('[]', ' ')).split()
Copy link
Contributor

Choose a reason for hiding this comment

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

What python version are you running? I tried this code with python 3.7 and got AttributeError: module 'string' has no attribute 'maketrans'. I also found this note on python release 3.1: The string.maketrans() function is deprecated and is replaced by new static methods, bytes.maketrans() and bytearray.maketrans().

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah let me check my setup



def get_io_scheduler(device_name):
"""
Return io scheduler name which is set currently for device
:param device_name: Device name example like sda , hda
:return: IO scheduler
:rtype : str
"""
return re.split(r'[\[\]]',
open(__sched_path(device_name)).read())[1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment for the context manager here.



def set_io_scheduler(device_name, name):
"""
Set io scheduler to a device
:param device_name: Device name example like sda , hda
:param name: io scheduler name
"""
if name not in get_io_scheduler_list(device_name):
raise DiskUtilsError('No such IO scheduler: %s' % name)

with open(__sched_path(device_name), 'w') as fp:
fp.write(name)


def __sched_path(device_name):
return '/sys/block/%s/queue/scheduler' % device_name