Bug 498398

Summary: [PATCH] [Refactoring] A series of refactoring patches to sos
Product: [Fedora] Fedora Reporter: Satoru SATOH <ssato>
Component: sosAssignee: Adam Stokes <adam.stokes>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: 11CC: adam.stokes, astokes
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 1.8-14.fc10 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-08-07 01:04:02 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Attachments:
Description Flags
Encapsulate the private field self.cInfo["policy"] with a method
none
Make the special method __init__ returns nothing
none
Make cluster.is_cluster_quorate() returns Bool always
none
Fix a potential bug in SosPolicy.allPkgsByName()
none
Extract a method to check if specific package is installed
none
Use PluginBase.policy and PluginBase.isInstalled instead of direct access to private field
none
Fix not to raise exception if tdstpath is not set none

Description Satoru SATOH 2009-04-30 06:08:32 EDT
Created attachment 341890 [details]
Encapsulate the private field self.cInfo["policy"] with a method

Description of problem:

This is not a bug or enhancement but a kind of refactoring.

I'm trying to enhance plugins/cluster.py to find clear problems in cluster configuration more aggressively and come across the followings.

 * Some unnecessary and potentially problematic lines
 * Inherited classes access to the private field in the parent classes
 * Some methods return inconsistent typed objects
 * Some private fields are directly access

A series of patches I'll post are correspnding fix for the above issues.
 


Version-Release number of selected component (if applicable):
sos-1.8-10.fc11.noarch
Comment 1 Satoru SATOH 2009-04-30 06:09:54 EDT
Created attachment 341891 [details]
Make the special method __init__ returns nothing
Comment 2 Satoru SATOH 2009-04-30 06:10:34 EDT
Created attachment 341892 [details]
Make cluster.is_cluster_quorate() returns Bool always
Comment 3 Satoru SATOH 2009-04-30 06:12:33 EDT
Created attachment 341893 [details]
Fix a potential bug in SosPolicy.allPkgsByName()

SosPolicy.allPkgsByName() returns None and this may cause TypeError in other methods such as pkgProvides and pkgRequires.
Comment 4 Satoru SATOH 2009-04-30 06:13:32 EDT
Created attachment 341894 [details]
Extract a method to check if specific package is installed
Comment 5 Satoru SATOH 2009-04-30 06:14:21 EDT
Created attachment 341896 [details]
Use PluginBase.policy and PluginBase.isInstalled instead of direct access to private field
Comment 6 Satoru SATOH 2009-04-30 06:23:18 EDT
Created attachment 341898 [details]
Fix not to raise exception if tdstpath is not set

This patch should suppress an exception UnBoundLocalError raised (see an example below):


[from sos_logs/sos.log]

2009-04-30 18:29:33,337 verbose2: error copying from pathspec /etc/xinetd.d (local variable 'tdstpath' reference
d before assignment), traceback follows:
2009-04-30 18:29:33,337 verbose2: Traceback (most recent call last):
  File "/usr/lib/python2.6/site-packages/sos/plugintools.py", line 490, in copyStuff
    self.doCopyFileOrDir(path)
  File "/usr/lib/python2.6/site-packages/sos/plugintools.py", line 207, in doCopyFileOrDir
    self.doCopyFileOrDir(srcpath+'/'+afile)
  File "/usr/lib/python2.6/site-packages/sos/plugintools.py", line 224, in doCopyFileOrDir
    self.copiedFiles.append({'srcpath':srcpath, 'dstpath':tdstpath, 'symlink':"no"}) # save in our list
UnboundLocalError: local variable 'tdstpath' referenced before assignment
Comment 7 Satoru SATOH 2009-04-30 06:26:16 EDT
sorry for a bunch of patches. Each of patches is tiny so that I posted them here as a whole.

If I need to open other bugzilla entries, let me know please.
Comment 8 Adam Stokes 2009-04-30 15:40:09 EDT
Hey Satoru,

Thanks for the patches I'll get these looked over and committed over the weekend.

Thanks!
Adam
Comment 9 Satoru SATOH 2009-05-01 04:49:58 EDT
Thanks for your quick response! 

If you have any asks or concerns, let me know, please.
Also, I'll try resolve issues asap if exists.

- satoru
Comment 10 Adam Stokes 2009-05-01 17:29:32 EDT
I've committed to trunk, do me a favor though and do some quick tests on these patches to make sure its working for you before I push out an update to testing.

Thanks!
Comment 12 Satoru SATOH 2009-05-17 03:35:49 EDT
sorry to late reply.

I checked out svn trunk and tried on both rhel-4 and rhel-5.
I found another problems (I'll report them as another bz entry later), though, 
it almost looks fine.

Thanks for your quick work!

- satoru
Comment 13 Bug Zapper 2009-06-09 10:51:57 EDT
This bug appears to have been reported against 'rawhide' during the Fedora 11 development cycle.
Changing version to '11'.

More information and reason for this action is here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping
Comment 14 Fedora Update System 2009-07-23 13:20:31 EDT
sos-1.8-14.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/sos-1.8-14.fc11
Comment 15 Fedora Update System 2009-07-23 13:22:05 EDT
sos-1.8-14.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/sos-1.8-14.fc10
Comment 16 Fedora Update System 2009-07-24 15:37:16 EDT
sos-1.8-14.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update sos'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-7976
Comment 17 Fedora Update System 2009-07-24 15:38:05 EDT
sos-1.8-14.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update sos'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-7976
Comment 18 Fedora Update System 2009-08-07 01:03:36 EDT
sos-1.8-14.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 19 Fedora Update System 2009-08-07 01:06:05 EDT
sos-1.8-14.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.