Avoid MD5 #1812
Avoid MD5 #1812
Conversation
| if not is_stream: | ||
| os.remove(filename) | ||
| raise MD5Error(filename) | ||
| if md5 and etag != md5.hexdigest(): |
There was a problem hiding this comment.
None is falsy, so is that necessary?
There was a problem hiding this comment.
Yes. Semantically, if you have a value of None, you need to differentiate between "provided a false like value" and "did not provide a value".
Also, from pep8:
Also, beware of writing if x when you really mean if x is not None -- e.g. when testing whether a variable or argument that defaults to None was set to some other value. The other value might have a type (such as a container) that could be false in a boolean context!
|
Looks good, just had some small comments. |
JordonPhillips
force-pushed
the
check-md5
branch
from
841652c to
6083393
Compare
| # Setup MD5 patches | ||
| self.md5_object = mock.Mock() | ||
| self.md5_object.hexdigest = mock.Mock(return_value=etag) | ||
| md5_builder = mock.Mock(return_value=self.md5_object) |
There was a problem hiding this comment.
Unless it's more convenient to store these as instance attributes you can also just say:
md5_builder = mock.Mock()
md5_builder.return_value.hexdigest.return_value = etag
That makes it more clear that you're only storing mocks that you plan on asserting things on later.
|
|
JordonPhillips
force-pushed
the
check-md5
branch
from
6083393 to
f98b684
Compare
Avoid MD5 if it isn't available.