20 + if added_list:
21 + for package_name in added_list.split(','):
22 + if package_name not in package_list:
23 + # Only add packages not already in the global list
24 + package_list.append(package_name)
25 +
26 + # Finally remove the list of removed binaries from the ticket request
27 + removed_list = request_parameters.get('removed_binaries', '')
28 + if removed_list:
29 + for package_name in removed_list.split(','):
30 + try:
31 + package_list.remove(package_name)
32 + except ValueError:
33 + # This is ok, multiple queued tickets could be trying
34 + # to remove the same binary.
35 + logger.info('Package to remove not found: '
36 + '{}'.format(package_list))
37 +
Not a big deal, and I could see some people advocate what you have there. But I often find it easier to use Python "sets" for things like this. I think you could simplify this to something like:
None of my comments are really a big deal as the code looks like it works to me. I'll just ack the MP and you can decide whether or not you want to do anything with my comments.
46 + packages = request_ parameters. get('added_ binaries' , '')
47 + if packages:
48 + packages = packages.split(',')
I don't think you need 47 & 48, python's split should be safe there.
63 + parameters = {'added_binaries': ','.join(packages),
64 + 'removed_binaries': None}
Bikeshedding, but most of our code is declaring dictionaries styled like:
parameters = { added_binaries' : ','.join(packages), removed_ binaries' : None
'
'
}
20 + if added_list: split(' ,'): list.append( package_ name) parameters. get('removed_ binaries' , '') list.split( ','): list.remove( package_ name) info('Package to remove not found: ' package_ list))
21 + for package_name in added_list.
22 + if package_name not in package_list:
23 + # Only add packages not already in the global list
24 + package_
25 +
26 + # Finally remove the list of removed binaries from the ticket request
27 + removed_list = request_
28 + if removed_list:
29 + for package_name in removed_
30 + try:
31 + package_
32 + except ValueError:
33 + # This is ok, multiple queued tickets could be trying
34 + # to remove the same binary.
35 + logger.
36 + '{}'.format(
37 +
Not a big deal, and I could see some people advocate what you have there. But I often find it easier to use Python "sets" for things like this. I think you could simplify this to something like:
packages = set(ticket. get_binaries( )) parameters. get('added_ binaries' , '').split(',')) parameters. get('removed_ binaries' , '').split(','))
packages = packages + set(request_
packages = packages - set(request_
None of my comments are really a big deal as the code looks like it works to me. I'll just ack the MP and you can decide whether or not you want to do anything with my comments.