Code review comment for lp:~mbp/launchpad/612171-diff-generation-spam

Revision history for this message
Martin Pool (mbp) wrote :

On 31 October 2011 14:19, Aaron Bentley <email address hidden> wrote:
> The meaning you give to tuples is not consistent with the way tuples
> are used in except clauses.  Except clauses use a tuple to specify a
> sequence of fixed but arbitrary length, in which all elements are
> treated the same.  The retry_error_types member is that portion of an
> except clause, extracted to a member variable.   isinstance has the
> same semantics.  So I think a tuple conforms to Python style, and a
> list varies from it.

One interesting thing here is that Python accepts

  except [IndexError, ValueError], e:

but it won't actually match them: I guess it may be looking for
old-style exceptions that raised a List or something. So I should
definitely fix that, and it's apparently not being tested enough.

>> I don't feel that getting protection against "accidental" mutation
>> is important here because there are so many other ways that can be
>> broken in Python, most obviously by just assigning to the attribute
>> on either the instance or the class.
>
> Python usually assumes that we're consenting adults, so there's very
> little you can't do, if you try.
>
> There's a big difference between "self.retry_error_types.append" and
> "self.retry_error_types = self.retry_error_types +".  The second will
> affect only the instance, while the first will affect all instances.
> You can affect all instances, if you want, but you have to spell that out.

I realize that.

« Back to merge proposal