[PATCH 0/2] Add some log.debug to help debugging

[Pierre-Yves Chibon]

Hi,

These two patches simply add some calls to log.debug() where exception are
catched so that we can find out what's going on.

Thanks,
Pierre



Pierre-Yves Chibon (2):
  Add a little debugging when pkg_exists fails
  Log a little more info if an AuthError is catched

 scripts/process-git-requests/process-git-requests | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

-- 
1.9.3

[PATCH 1/2] Add a little debugging when pkg_exists fails

[Pierre-Yves Chibon]

---
 scripts/process-git-requests/process-git-requests | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/process-git-requests/process-git-requests
b/scripts/process-git-requests/process-git-requests
index d12e419..6c41b4b 100755
--- a/scripts/process-git-requests/process-git-requests
+++ b/scripts/process-git-requests/process-git-requests
@@ -147,7 +147,8 @@ def add_package(pkgdb, request, bug):
     def pkg_exists(pkg):
         try:
             pkgdb.get_package(str(pkg))
-        except PkgDBException:
+        except PkgDBException, err:
+            log.debug('Error when retrieving package %s: %s' % (str(pkg), err))
             # We get an error back if (at least) the package doesn't exist.
             return False
         return True
-- 
1.9.3

[PATCH 1/2] Add a little debugging when pkg_exists fails

[Mathieu Bridon]

On Mon, 2014-11-03 at 18:00 +0100, Pierre-Yves Chibon wrote:
>  ---
>   scripts/process-git-requests/process-git-requests | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>  
>  diff --git a/scripts/process-git-requests/process-git-requests
> b/scripts/process-git-requests/process-git-requests
>  index d12e419..6c41b4b 100755
>  --- a/scripts/process-git-requests/process-git-requests
>  +++ b/scripts/process-git-requests/process-git-requests
>  @@ -147,7 +147,8 @@ def add_package(pkgdb, request, bug):
>       def pkg_exists(pkg):
>           try:
>               pkgdb.get_package(str(pkg))
>  -        except PkgDBException:
>  +        except PkgDBException, err: 
Not necessary for this change, but you should try getting into the habit
of writing this like:

>  +        except PkgDBException as err: 
That will make it easier when we one day decide to port to Python 3. :)

The infra runs on Python >= 2.6, where this syntax is always available.


-- 
Mathieu

[PATCH 1/2] Add a little debugging when pkg_exists fails

[Pierre-Yves Chibon]

---
 scripts/process-git-requests/process-git-requests | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/process-git-requests/process-git-requests
b/scripts/process-git-requests/process-git-requests
index c50f608..bdf4dff 100755
--- a/scripts/process-git-requests/process-git-requests
+++ b/scripts/process-git-requests/process-git-requests
@@ -147,7 +147,8 @@ def add_package(pkgdb, request, bug):
     def pkg_exists(pkg):
         try:
             pkgdb.get_package(str(pkg))
-        except PkgDBException:
+        except PkgDBException as err:
+            log.debug('Error when retrieving package %s: %s', str(pkg), err)
             # We get an error back if (at least) the package doesn't exist.
             return False
         return True
-- 
1.9.3

[PATCH 2/2] Log a little more info if an AuthError is catched

[Pierre-Yves Chibon]

---
 scripts/process-git-requests/process-git-requests | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/process-git-requests/process-git-requests
b/scripts/process-git-requests/process-git-requests
index bdf4dff..a4230de 100755
--- a/scripts/process-git-requests/process-git-requests
+++ b/scripts/process-git-requests/process-git-requests
@@ -189,6 +189,7 @@ def add_package(pkgdb, request, bug):
                     'Approved',
                     i)
         except AuthError, e:
+            logging.debug('It says AuthError: %s', e)
             if sys.stdin.isatty():
                 if retry >= config['pkgdb.retries']:
                     break
-- 
1.9.3

[PATCH] Expand the logging in process-git-requests

[Pierre-Yves Chibon]

Add the --verbose argument
Make the --debug argument do something (including incorporating the logs
from pkgdb2client itself)
Add more call to logging.log()
---
 scripts/process-git-requests/process-git-requests | 38 +++++++++++++++++++----
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/scripts/process-git-requests/process-git-requests
b/scripts/process-git-requests/process-git-requests
index a4230de..95e8e61 100755
--- a/scripts/process-git-requests/process-git-requests
+++ b/scripts/process-git-requests/process-git-requests
@@ -55,6 +55,8 @@ PAGER = os.environ.get('PAGER') or '/usr/bin/less'
 EDITOR = os.environ.get('EDITOR') or '/bin/vi'
 
 logging.basicConfig()
+PKGDBLOG = logging.getLogger("pkgdb2client")
+LOG = logging.getLogger("process-git-requests")
 
 # Override a method in xmlrpclib so it doesn't blow up when getting crap data
 # from Red Hat's bugzilla.
@@ -88,6 +90,11 @@ def parse_commandline():
             help='Username for PackageDB connection',
             dest='user',
             default=getpass.getuser())
+    parser.add_option('--verbose',
+            action='store_true',
+            dest='verbose',
+            default=False,
+            help='Outputs more information (less than debug though)')
     parser.add_option('--debug',
             action='store_true',
             dest='debug',
@@ -136,11 +143,13 @@ def encode_utf8(object, encoding='utf8',
errors='replace'):
 # Get all valid branches for a package:
 # pkg_branches :: PkgDB -> String -> (IO) [String]
 def pkg_branches(pkgdb, pkg):
+    LOG.info('Getting branches of package %s from pkgdb', pkg)
     package = pkgdb.get_package(str(pkg))
     valid_branches = filter(lambda x: x['collection']['branchname'] in
branches.keys(), package['packages'])
     return map(lambda x: x['collection']['branchname'], valid_branches)
 
 def add_package(pkgdb, request, bug):
+    LOG.info('Adding package %s in pkgdb', request['pkg'])
 
     # A nested helper function to see if a package exists or not.
     # pkg_exists :: String -> (IO) Bool
@@ -148,7 +157,7 @@ def add_package(pkgdb, request, bug):
         try:
             pkgdb.get_package(str(pkg))
         except PkgDBException as err:
-            log.debug('Error when retrieving package %s: %s', str(pkg), err)
+            LOG.debug('Error when retrieving package %s: %s', str(pkg), err)
             # We get an error back if (at least) the package doesn't exist.
             return False
         return True
@@ -189,7 +198,7 @@ def add_package(pkgdb, request, bug):
                     'Approved',
                     i)
         except AuthError, e:
-            logging.debug('It says AuthError: %s', e)
+            LOG.debug('It says AuthError: %s', e)
             if sys.stdin.isatty():
                 if retry >= config['pkgdb.retries']:
                     break
@@ -201,6 +210,7 @@ def add_package(pkgdb, request, bug):
             break
 
 def edit_package(pkgdb, request):
+    LOG.info('Edit package %s in pkgdb', request['pkg'])
     for retry in range(1, config['pkgdb.retries'] + 1):
         try:
             for i in ([request['owner']] + request['comaintainers']):
@@ -219,7 +229,7 @@ def edit_package(pkgdb, request):
 
 def run_query(bz):
     querydata = {}
-    querydata['column_list'] = ['id', 'creation_time', 
+    querydata['column_list'] = ['id', 'creation_time',
             'assigned_to', 'reporter', 'bug_status',
'resolution',
             'component', 'blockedby', 'dependson',
'summary',
             'status_whiteboard', 'flags']
@@ -319,6 +329,7 @@ def parse_prefixed_lines(s):
 
     return items
 
+
 def clean_branches(branches):
     '''Clean up a list of branches and turn them into what pkgdb
expects.'''
     branches = branches.lower()
@@ -352,11 +363,11 @@ def clean_request(items):
     branches = clean_branches(items['Branches'].strip())
     branches += ' master'
     items['Branches'] = branches
-    request['branches'] = branches.split() 
+    request['branches'] = branches.split()
 
     branches = clean_branches(items['New Branches'].strip())
     items['New Branches'] = branches
-    request['newbranches'] = branches.split() 
+    request['newbranches'] = branches.split()
 
     owners = items['Owners'].strip()
     owners = re.sub(r',', ' ', owners)
@@ -379,6 +390,7 @@ def clean_request(items):
 
     return request
 
+
 def new_request_string(items, bug):
     r = []
     r.append("Bug URL: https://bugzilla.redhat.com/%d " % bug.id)
@@ -395,6 +407,7 @@ def new_request_string(items, bug):
     r.append('')
     return '\n'.join(r)
 
+
 def change_request_string(items, bug):
     r = []
     r.append("Bug URL: https://bugzilla.redhat.com/%d" % bug.id)
@@ -409,6 +422,7 @@ def change_request_string(items, bug):
     r.append('')
     return '\n'.join(r)
 
+
 def get_pkgdb_owners(pkgdb, pkg):
     owners = {}
     o = ''
@@ -442,6 +456,7 @@ def get_pkgdb_owners(pkgdb, pkg):
 
     return (owners, o)
 
+
 def process_no_request(bug, allcomments):
     '''Deal with a ticket where no request was found.'''
     while 1:
@@ -471,6 +486,7 @@ def process_no_request(bug, allcomments):
             webbrowser.open("https://bugzilla.redhat.com/" + str(bug.id),
new=1)
     return True
 
+
 def check_owners(fas, owner, comaintainers, cc_list):
     print "Checking owners..."
     warnings = []
@@ -502,11 +518,12 @@ def check_owners(fas, owner, comaintainers, cc_list):
             break
 
     return warnings
-        
+
 
 def process_new_request(bug, comment, allcomments, firstfound, pkgdb, fas, branches):
     '''Parse a new package request, try to repair line wrapping, and do some
     basic validity checks.'''
+    LOG.info('Processing new package requests')
     warned = False
     warnings = []
     items = parse_prefixed_lines(comment['text'])
@@ -652,6 +669,7 @@ def process_new_request(bug, comment, allcomments, firstfound, pkgdb,
fas, branc
 def process_change_request(bug, comment, allcomments, firstfound, pkgdb, branches):
     '''Parse a change request, try to repair line wrapping, and do some
     basic validity checks.'''
+    LOG.info('Processing new branch requests')
     owned = False
     warned = False
     warnings = []
@@ -786,6 +804,7 @@ def process_change_request(bug, comment, allcomments, firstfound,
pkgdb, branche
 def create_branches(package, pkghost, pkghostlocal, processed):
     '''If on pkgs01, run /usr/local/bin/pkgdb2branch.py directly. 
Otherwise,
     call ssh.'''
+    LOG.info('Create branches in git')
     hostname = os.uname()[1]
     cmd = []
     if hostname != pkghostlocal:
@@ -799,10 +818,17 @@ def create_branches(package, pkghost, pkghostlocal, processed):
     print
     return processed
 
+
 if __name__ == '__main__':
     processed = []
     options = parse_commandline()
 
+    if options.debug:
+        LOG.setLevel(logging.DEBUG)
+        PKGDBLOG.setLevel(logging.DEBUG)
+    elif options.verbose:
+        LOG.setLevel(logging.INFO)
+
     print "Connecting to bugzilla..."
     try:
         bz = bugzilla.Bugzilla(url=options.url)
-- 
1.9.3

[PATCH 2/2] Log a little more info if an AuthError is catched

[Pierre-Yves Chibon]

---
 scripts/process-git-requests/process-git-requests | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/process-git-requests/process-git-requests
b/scripts/process-git-requests/process-git-requests
index 6c41b4b..4f14a5b 100755
--- a/scripts/process-git-requests/process-git-requests
+++ b/scripts/process-git-requests/process-git-requests
@@ -189,6 +189,7 @@ def add_package(pkgdb, request, bug):
                     'Approved',
                     i)
         except AuthError, e:
+            logging.debug('It says AuthError: %s' % e)
             if sys.stdin.isatty():
                 if retry >= config['pkgdb.retries']:
                     break
-- 
1.9.3

[PATCH 2/2] Log a little more info if an AuthError is catched

[Till Maas]

On Mon, Nov 03, 2014 at 06:00:25PM +0100, Pierre-Yves Chibon wrote:
>  ---
>   scripts/process-git-requests/process-git-requests | 1 +
>   1 file changed, 1 insertion(+)
>  
>  diff --git a/scripts/process-git-requests/process-git-requests
> b/scripts/process-git-requests/process-git-requests
>  index 6c41b4b..4f14a5b 100755
>  --- a/scripts/process-git-requests/process-git-requests
>  +++ b/scripts/process-git-requests/process-git-requests
>  @@ -189,6 +189,7 @@ def add_package(pkgdb, request, bug):
>                       'Approved',
>                       i)
>           except AuthError, e:
>  +            logging.debug('It says AuthError: %s' % e) 
Just a small remark: It is more efficient to write

|              logging.debug('It says AuthError: %s', e)

because then the "%" operator is not used when debug messages are not
printed.

Regards
Till