-
Notifications
You must be signed in to change notification settings - Fork 286
Put Md5 with UT #3175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Put Md5 with UT #3175
Conversation
SophieGuo410
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to make sure:
- the id converter in close source also gets update.
- we need to make sure all the db has this digest field before we merge this pr.
| deleted_ts datetime(6) DEFAULT NULL, | ||
| blob_size bigint unsigned DEFAULT 0, | ||
| modified_ts datetime(6) DEFAULT CURRENT_TIMESTAMP(6), | ||
| digest varchar(255) DEFAULT NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need 255 length right? VARCHAR(50) is enough
| * Test put Named Blob with digest. | ||
| */ | ||
| @Test | ||
| public void putNamedBlobDigestTest() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we also have some test in MySqlNamedBlobDbIntegrationTest to make sure the mysql query works fine.
| * @param version the version of this named blob. | ||
| * @param digest the digest of this blob | ||
| */ | ||
| public NamedBlobRecord(String accountName, String containerName, String blobName, String blobId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like we have too many constructors which might not nessessary, we can try to combine them. we can have a follow up pr to do it.
Summary
Put Md5 for caspian migration
Testing Done
unit test