Code review comment for lp:~jaypipes/glance/checksum

Revision history for this message
Rick Harris (rconradharris) wrote :

Nice patch.

Unfortunately with the state of Nova <=> Glance trunks at the moment, I wasn't able to test functionally. That said, this seems pretty safe to go ahead and merge, and if we have issues, we can fix them along with all the other fixes that are in progress at the moment :)

> 110 + checksum = Column(String(32))

I assume the answer is YAGNI, but I'll ask it anyway: Will we ever allow SHA1 or SHA256?

Would it make sense to make this a String(64) or, heck, a String(255) for flexibility?

review: Approve

« Back to merge proposal