openstack team mailing list archive

[Andrew Clay Shafer <acs@xxxxxxxxxxxxxxxx>]

Pete,

There is clearly something interesting going on with scope. 'options',
which appears to really be 'parser' get passed in as a variable, but are
then gets over written before being used by the call using the global
'parser'.
(options, args) = parse_args(parser, args)

The history of the commands you run and the stacktrace from the errors/logs
might help.

What were you expecting/trying to do with the code change?

Most of that code looks like it was written by Greg Holt and/or Chuck Thier.

Hopefully they can shed more light on this by eyeballing the code than I
can.




On Thu, Apr 5, 2012 at 8:43 PM, Pete Zaitcev <zaitcev@xxxxxxxxxx> wrote:

> Hi, All:
>
> In the process of tinkering for lp:959221, I made a benign modification
> to make it possible to invoke swift as a module, when everything broke
> loose with errors like "NameError: global name 'parser' is not defined".
> Looking at the code it seems like a thinko, with the following fix:
>
> diff --git a/bin/swift b/bin/swift
> index 0cac5d6..a14c646 100755
> --- a/bin/swift
> +++ b/bin/swift
> @@ -1202,7 +1203,7 @@ download --all OR download container [options]
> [object] [object] ...
>     stdout.'''.strip('\n')
>
>
> -def st_download(options, args, print_queue, error_queue):
> +def st_download(parser, args, print_queue, error_queue):
>     parser.add_option('-a', '--all', action='store_true', dest='yes_all',
>         default=False, help='Indicates that you really want to download '
>         'everything in the account')
> @@ -1378,7 +1379,7 @@ list [options] [container]
>  '''.strip('\n')
>
>
> -def st_list(options, args, print_queue, error_queue):
> +def st_list(parser, args, print_queue, error_queue):
>     parser.add_option('-p', '--prefix', dest='prefix', help='Will only
> list '
>         'items beginning with the prefix')
>     parser.add_option('-d', '--delimiter', dest='delimiter', help='Will
> roll '
> @@ -1423,7 +1424,7 @@ stat [container] [object]
>     args given (if any).'''.strip('\n')
>
>
> -def st_stat(options, args, print_queue, error_queue):
> +def st_stat(parser, args, print_queue, error_queue):
>     (options, args) = parse_args(parser, args)
>     args = args[1:]
>     conn = get_conn(options)
> @@ -1548,7 +1549,7 @@ post [options] [container] [object]
>     post -m Color:Blue -m Size:Large'''.strip('\n')
>
>
> -def st_post(options, args, print_queue, error_queue):
> +def st_post(parser, args, print_queue, error_queue):
>     parser.add_option('-r', '--read-acl', dest='read_acl', help='Sets the '
>         'Read ACL for containers. Quick summary of ACL syntax: .r:*, '
>         '.r:-.example.com, .r:www.example.com, account1, account2:user2')
> @@ -1619,7 +1620,7 @@ upload [options] container file_or_directory
> [file_or_directory] [...]
>  '''.strip('\n')
>
>
> -def st_upload(options, args, print_queue, error_queue):
> +def st_upload(parser, args, print_queue, error_queue):
>     parser.add_option('-c', '--changed', action='store_true',
> dest='changed',
>         default=False, help='Will only upload files that have changed
> since '
>         'the last upload')
>
> Seems obvious if I look at this:
>
>    globals()['st_%s' % args[0]](parser, argv[1:], print_queue, error_queue)
>
> The parser is always the first argument, so... Someone please tell me
> if I am missing something here. Or should I just file it in Gerrit?
>
> Weird part is, unit tests pass both on the current code and the one
> with my patch above. The problem seems too gross to let it work, but
> everything appears in order.
>
> -- Pete
>
> _______________________________________________
> Mailing list: https://launchpad.net/~openstack
> Post to     : openstack@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~openstack
> More help   : https://help.launchpad.net/ListHelp
>