[PATCH] Check that opts is not None before accessing.

[Ralph Bean]

This should fix the periodic 500 error we get from koji-web in Fedora
Infra's staging environment.

Signed-off-by: Ralph Bean 
---
 www/lib/kojiweb/util.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/lib/kojiweb/util.py b/www/lib/kojiweb/util.py
index 89a5ddb..6610beb 100644
--- a/www/lib/kojiweb/util.py
+++ b/www/lib/kojiweb/util.py
@@ -466,7 +466,7 @@ def taskScratchClass(task_object):
     request = task_object['request']
     if len(request) >= 3:
         opts = request[2]
-        if opts.get('scratch'):
+        if opts and opts.get('scratch'):
             return "scratch"
     return ""

[Ralph Bean]

On Fri, May 08, 2015 at 11:56:40AM -0400, Ralph Bean wrote:
> This should fix the periodic 500 error we get from koji-web in Fedora
> Infra's staging environment.
> Signed-off-by: Ralph Bean
>
> www/lib/kojiweb/util.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/www/lib/kojiweb/util.py b/www/lib/kojiweb/util.py
> index 89a5ddb..6610beb 100644
> --- a/www/lib/kojiweb/util.py
> +++ b/www/lib/kojiweb/util.py
> @@ -466,7 +466,7 @@ def taskScratchClass(task_object):
>      request = task_object['request']
>      if len(request) >= 3:
>          opts = request[2]
>    if opts.get('scratch'):
>    if opts and opts.get('scratch'):
>        return "scratch"
> return ""
>
> -- 
> 2.1.0

FYI, we miscommunicated about this in #fedora-releng yesterday and the
unpatched version got pushed out to prod.  This means that every now
and then, the koji frontpage fails with 500 (until the task with
`opts == None` scrolls off the "Recent Tasks" pane).

[Kevin Fenzi]

On Sat, 9 May 2015 08:25:41 -0400
Ralph Bean  wrote:
> On Fri, May 08, 2015 at 11:56:40AM -0400, Ralph Bean wrote:
> > This should fix the periodic 500 error we get from koji-web in
> > Fedora Infra's staging environment.
> > Signed-off-by: Ralph Bean
> >
> > www/lib/kojiweb/util.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/www/lib/kojiweb/util.py b/www/lib/kojiweb/util.py
> > index 89a5ddb..6610beb 100644
> > --- a/www/lib/kojiweb/util.py
> > +++ b/www/lib/kojiweb/util.py
> > @@ -466,7 +466,7 @@ def taskScratchClass(task_object):
> >      request = task_object['request']
> >      if len(request) >= 3:
> >          opts = request[2]
> >    if opts.get('scratch'):
> >    if opts and opts.get('scratch'):
> >        return "scratch"
> > return ""
> >
> > -- 
> > 2.1.0
>
> FYI, we miscommunicated about this in #fedora-releng yesterday and the
> unpatched version got pushed out to prod.  This means that every now
> and then, the koji frontpage fails with 500 (until the task with
> `opts == None` scrolls off the "Recent Tasks" pane).

+1 for hotfixing it now and seeing if we can roll it into a new release
monday before freeze. ;)

kevin

[PATCH] Be still more careful with "opts" in koji-web.

[Ralph Bean]

And add some comments to this section so that future readers will understand
all the gymnastics.
---
 www/lib/kojiweb/util.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/www/lib/kojiweb/util.py b/www/lib/kojiweb/util.py
index 6610beb..7f7b578 100644
--- a/www/lib/kojiweb/util.py
+++ b/www/lib/kojiweb/util.py
@@ -463,10 +463,16 @@ def taskScratchClass(task_object):
     """ Return a css class indicating whether or not this task is a scratch
     build.
     """
+    # Here, we try to figure out if this is supposed to be a scratch task based
+    # on this 'request' list attached to the task object.  It's hard to know
+    # exactly what's in it, because it's an unstructured list.  Different kinds
+    # of tasks stuff different things in it in different places.
     request = task_object['request']
     if len(request) >= 3:
         opts = request[2]
-        if opts and opts.get('scratch'):
+        # This is tough, because "opts" could be a one of a number of different
+        # things.  A dict, a bool, None, etc..
+        if hasattr(opts, 'get') and opts.get('scratch'):
             return "scratch"
     return ""

[PATCH] Be still more careful with "opts" in koji-web.

[Mike McLean]

On 05/09/2015 07:36 PM, Ralph Bean wrote:
> And add some comments to this section so that future readers will understand
> all the gymnastics.
>
> www/lib/kojiweb/util.py | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/www/lib/kojiweb/util.py b/www/lib/kojiweb/util.py
> index 6610beb..7f7b578 100644
> --- a/www/lib/kojiweb/util.py
> +++ b/www/lib/kojiweb/util.py
> @@ -463,10 +463,16 @@ def taskScratchClass(task_object):
>       """ Return a css class indicating whether or not this task is a scratch
>       build.
>       """
> # Here, we try to figure out if this is supposed to be a scratch task based
> # on this 'request' list attached to the task object.  It's hard to know
> # exactly what's in it, because it's an unstructured list.  Different kinds
> # of tasks stuff different things in it in different places.

Sorry I hadn't gotten around to reviewing the earlier patch.

If you're going to dig through the task request, you should most likely 
be checking the task method first. Checking for a minimum request length 
is not really a sufficient check.

Every task method has its own signature, some of which have changed over 
time.
>   request = task_object['request']
>   if len(request) >= 3:
>       opts = request[2]
>    if opts and opts.get('scratch'):
>    # This is tough, because "opts" could be a one of a number of different
>    # things.  A dict, a bool, None, etc..
>    if hasattr(opts, 'get') and opts.get('scratch'):
>         return "scratch"
> return ""

[PATCH] Check task method before trying to determine "scratch" status.

[Ralph Bean]

As per feedback here:  https://lists.fedoraproject.org/pipermail/buildsys/2015-May/004684.html

Signed-off-by: Ralph Bean 
---
 www/lib/kojiweb/util.py | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/www/lib/kojiweb/util.py b/www/lib/kojiweb/util.py
index 7f7b578..214b586 100644
--- a/www/lib/kojiweb/util.py
+++ b/www/lib/kojiweb/util.py
@@ -463,15 +463,13 @@ def taskScratchClass(task_object):
     """ Return a css class indicating whether or not this task is a scratch
     build.
     """
-    # Here, we try to figure out if this is supposed to be a scratch task based
-    # on this 'request' list attached to the task object.  It's hard to know
-    # exactly what's in it, because it's an unstructured list.  Different kinds
-    # of tasks stuff different things in it in different places.
+    method = task_object['method']
     request = task_object['request']
-    if len(request) >= 3:
+    if method == 'build' and len(request) >= 3:
+        # Each task method has its own signature for what gets put in the
+        # request list.  Builds should have an `opts` dict at index 2.
+        # See www/kojiweb/taskinfo.chtml for the grimoire.
         opts = request[2]
-        # This is tough, because "opts" could be a one of a number of different
-        # things.  A dict, a bool, None, etc..
         if hasattr(opts, 'get') and opts.get('scratch'):
             return "scratch"
     return ""

[PATCH] Check task method before trying to determine "scratch" status.

[Dennis Gilmore]

I have rebased and applied this patch

Dennis

On Monday, May 11, 2015 02:02:18 PM Ralph Bean wrote:
> As per feedback
> here:  https://lists.fedoraproject.org/pipermail/buildsys/2015-May/004684.h
> tml
> Signed-off-by: Ralph Bean
>
> www/lib/kojiweb/util.py | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/www/lib/kojiweb/util.py b/www/lib/kojiweb/util.py
> index 7f7b578..214b586 100644
> --- a/www/lib/kojiweb/util.py
> +++ b/www/lib/kojiweb/util.py
> @@ -463,15 +463,13 @@ def taskScratchClass(task_object):
>      """ Return a css class indicating whether or not this task is a scratch
> build.
>      """
> # Here, we try to figure out if this is supposed to be a scratch task
>
> based -    # on this 'request' list attached to the task object.  It's hard
> to know -    # exactly what's in it, because it's an unstructured
> list.  Different kinds -    # of tasks stuff different things in it in
> different places.
> method = task_object['method']
> request = task_object['request']
> if len(request) >= 3:
> if method == 'build' and len(request) >= 3:
>    # Each task method has its own signature for what gets put in the
>    # request list.  Builds should have an `opts` dict at index 2.
>    # See www/kojiweb/taskinfo.chtml for the grimoire.
>    opts = request[2]
>    # This is tough, because "opts" could be a one of a number of
>
> different -        # things.  A dict, a bool, None, etc..
>          if hasattr(opts, 'get') and opts.get('scratch'):
>              return "scratch"
>      return ""
> -- 
> 2.1.0