Re: [Fuego] UnicodeEncodeError fix (was RE: selftest instructions)

Thank you for looking into this! Looks good to me!

Best,

Lazlo

On Fri, Jul 17, 2020 at 12:37 AM Bird, Tim <[email protected]> wrote:

> > -----Original Message-----
> > From: Laszlo Sitzer
> >
> > Hello!
> >
> > I am working on updating the Fuego docker image and want to make sure I
> don't break anything. I notice in Fuego v1.3 a self-test was
> > introduced.
> >
> > I wanted to check if the following sequence is sufficient to test Fuego
> works correctly:
> >
> > >>>
> > ftc add-nodes -b docker
> > ftc add-jobs -b docker -t Functional.fuego_release_test
> > ftc run-test -b docker -t Functional.fuego_release_test
> > >>>
>
> I know that the UnicodeEncodeError exception that was raised during 'ftc
> add-nodes -b docker'
> was not the real error here.  That was an error that was covering up the
> real error
> from Jenkins (which is now fixed with a hardcoded plugin version in the
> Dockerfile).
> However, I have made a patch to work around the UnicodeEncodeError.
> It required a bit more work than I expected, and uncovered some
> controversial decisions
> by the python community in the Python 2.X lifecycle.
>
> In any event, I'm inlining the patch below, which is now already committed
> to the
> Fuego master branch.  In my testing here, it avoids masking errors from
> Jenkins.
>
> If someone sees some problem with this, let me know.
>
> Thanks very much to Laszlo Sitzer for bringing this to my attention.
>  -- Tim
>
> patch follows:
> -----------------
> From 600c12ffc55137aa31da709cfe3ed95d8853e76c Mon Sep 17 00:00:00 2001
> From: Tim Bird <[email protected]>
> Date: Thu, 16 Jul 2020 13:23:28 -0600
> Subject: [PATCH] ftc: handle UnicodeEncodeError from jenkins exceptions
>
> A cascade exception ('UnicodeEncodeError') was preventing
> user visibility on errors from the Jenkins server.
>
> Change the error handling so that if we get a UnicodeEncodeError
> when trying to convert an exception into a string, we change
> the default encoding for python string conversions from 'ascii'
> to 'utf8'.  A description of the issues is in the comment
> in the code.
>
> If feels like a hack, but it gets the job done!
>
> Thanks to Laszlo Sitzer for the bug report.
>
> Signed-off-by: Tim Bird <[email protected]>
> ---
>  scripts/ftc | 44 ++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/ftc b/scripts/ftc
> index 1b8ec02..c5e11ba 100755
> --- a/scripts/ftc
> +++ b/scripts/ftc
> @@ -5278,6 +5278,42 @@ def do_power_control(conf, options, command):
>              error_out("BOARD_CONTROL value %s is not supported" %
> board_control)
>      sys.exit(0)
>
> +# this takes an exception object (e), and formats it as a string
> +# safely (handling UnicodeEncode errors), then shows usage help, and
> +# exits the program with an error message.
> +#
> +# Background:
> +# python 2.7 uses default encoding of 'ascii'. If e is an exception from
> +# the jenkins module, then using str(e) will cause the module to use str()
> +# on the error message from the jenkins server. If that message has
> non-ascii
> +# chars (which is not uncommon), then python raises a UnicodeEncodeError
> +# exception.  I don't want to change the code for the jenkins
> +# module, and I have no other way of retrieving the string, which I'd like
> +# to print.
> +# So, do this hack where we reload sys and change the defaultencoding from
> +# 'ascii' to 'utf8'.  The function setdefaultencoding() exists in the
> +# sys module, but is deleted when it is imported (but not on 'reload').
> +# I could have modified /usr/lib/python2.7/site.py inside the container
> +# to remove the code that deletes the function.  This would have avoided
> +# the need to do the 'reload(sys)'.
> +# But I decided to limit the scope of this change.
> +# Sheesh - what a mess!
> +def safe_str_help_and_exit(e, ftc_cmd):
> +    try:
> +       err_msg = str(e)
> +       prefix = ""
> +    except UnicodeEncodeError:
> +        if sys.version_info[0] < 3:
> +           # this reload is hacky and can have bad side effects, but
> we're on
> +           # our way out of the program, so we don't care
> +           reload(sys)
> +           sys.setdefaultencoding('utf8')
> +
> +        err_msg = str(e)
> +        prefix = "ERROR: Operation '%s' raised exception with string:\n"
> % ftc_cmd
> +    msg = prefix + err_msg + '\n---\n' + command_help[ftc_cmd][1]
> +    sys.exit(msg)
> +
>  def main():
>      # use global module names
>      global re, time, copy2, subprocess, signal, fcntl, requests, json
> @@ -5534,7 +5570,7 @@ def main():
>              do_add_jobs(conf, options)
>          except Exception as e:
>              # this assumes the problem is something with the options
> -            sys.exit(str(e) + '\n' + command_help['add-jobs'][1])
> +            safe_str_help_and_exit(e, 'add-jobs')
>
>      if command.startswith("rm-job"):
>          # removes Jenkins jobs
> @@ -5542,7 +5578,7 @@ def main():
>          try:
>              do_rm_jobs(conf, options)
>          except Exception as e:
> -            sys.exit(str(e) + '\n' + command_help['rm-jobs'][1])
> +            safe_str_help_and_exit(e, 'rm-jobs')
>
>      if command.startswith("add-node"):
>          # adds Jenkins nodes
> @@ -5550,7 +5586,7 @@ def main():
>          try:
>              do_add_nodes(conf, options)
>          except Exception as e:
> -            sys.exit(str(e) + '\n' + command_help['add-nodes'][1])
> +            safe_str_help_and_exit(e, 'add-nodes')
>
>      if command.startswith("rm-node"):
>          # removes Jenkins nodes
> @@ -5558,7 +5594,7 @@ def main():
>          try:
>              do_rm_nodes(conf, options)
>          except Exception as e:
> -            sys.exit(str(e) + '\n' + command_help['rm-nodes'][1])
> +            safe_str_help_and_exit(e, 'rm-nodes')
>
>      if command == "list-nodes":
>          # shows jenkins nodes
> --
> 2.1.4
>
>
>