Change sync code to use new model #77
Conversation
Adds some fields that were necessary for sync, syncs in the new way, and heavily modify docblocks to explain. closes pulp#135
| author = StringField() | ||
| filename = StringField(required=True) | ||
| md5_digest = StringField() |
There was a problem hiding this comment.
I'm not crazy about having the md5_digest and _checksum and _checksum_type.
While you're in here, can we change this to a dictionary called checksums or something where the keys are the checksum algorithm and the value is the hex digest? I think this is also happening in the pulp_rpm plugin so if you do this make sure they match. It's not as nice as unifying our file and checksum modeling, but it'll be consistent at least.
There was a problem hiding this comment.
The reason I added this field is because _checksum and _checksum_type are both "Pulp fields", but md5_digest is a pure "Python field" in that it is a mirror of the field from PyPI. We generate our own checksum for later (not md5). I thought that it seemed like it made sense to save the PyPI checksum as well as our own, so that the client would be in a position to check that it matches both. Additionally, looking forward, when we are doing Pulp-Pulp syncs we can use either checksum, but I at least wanted the option of being able to naively syncing from another Pulp server without having to do anything different than we would I we were syncing from PyPI.
That all said, I think that it would be possible (and acceptable) to remove the md5_digest and save only a single checksum onto the object. I would like to have the pulp-pulp sync working before spending too much effort on that decision. Lets make sure to have a discussion before the final merge of the feature branch.
|
Just the one comment. Since this is going into a feature branch and will be reviewed as a whole, I didn't do a complete examination to make sure you didn't miss a |
Adds some fields that were necessary for sync, syncs in the new way, and
heavily modify docblocks to explain.
https://pulp.plan.io/issues/135