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

replace sgdisk subprocess calls with a helper #932

Merged
merged 4 commits into from Dec 13, 2013
Merged

replace sgdisk subprocess calls with a helper #932

merged 4 commits into from Dec 13, 2013

Conversation

alfredodeza
Copy link
Contributor

This fixes issue 6979, but most importantly adds a couple of things to ceph-disk that should get used a bit more and hopefully used in the other ceph-* scripts.

  • There was too much repetition for subprocess.* calls that were the same. By introducing a helper the repetition gets reduced and we now can handle errors for subprocess calls in one place.
  • Adds 2 Exceptions, because having custom messaging in the error was needed (something that the existing ones did not provide)
  • The try/except block at the end now just raises SystemExit vs. printing to stderr and then calling sys.exit(1). Raising that exception achieves the same functionality and it is just one call and easier to read.

This is how the error looks before these changes:

vagrant@node1:~$ ceph-disk list
Traceback (most recent call last):
  File "/usr/sbin/ceph-disk", line 2342, in <module>
    main()
  File "/usr/sbin/ceph-disk", line 2331, in main
    args.func(args)
  File "/usr/sbin/ceph-disk", line 2008, in main_list
    part_uuid = get_partition_uuid(dev)
  File "/usr/sbin/ceph-disk", line 1918, in get_partition_uuid
    stderr = subprocess.PIPE).stdout.read()
  File "/usr/lib/python2.7/subprocess.py", line 679, in __init__
    errread, errwrite)
  File "/usr/lib/python2.7/subprocess.py", line 1249, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

This is how it looks now:

vagrant@node1:~$ ceph-disk list
ceph-disk ExecutableNotFound: sgdisk not in path. Could not run command: sgdisk -i 1 /dev/sda

Signed-off-by: Alfredo Deza <alfredo@deza.pe>
for location in locations:
executable_path = os.path.join(location, executable)
if os.path.exists(executable_path):
return executable_path
Copy link
Member

Choose a reason for hiding this comment

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

we are just assuming that this matches the search path? would it make more sense to return the match and use it explicitly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, that sounds reasonable

Signed-off-by: Alfredo Deza <alfredo@deza.pe>
Signed-off-by: Alfredo Deza <alfredo@deza.pe>
Signed-off-by: Alfredo Deza <alfredo@deza.pe>
liewegas pushed a commit that referenced this pull request Dec 13, 2013
replace sgdisk subprocess calls with a helper

Reviewed-by: Sage Weil <sage@inktank.com>
@liewegas liewegas merged commit e7652e6 into master Dec 13, 2013
@liewegas liewegas deleted the wip-6979 branch December 13, 2013 18:03
liewegas added a commit that referenced this pull request Dec 14, 2016
morepggrow: enable filestore and journal soft backoffs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants