Skip to content
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

[728-1] Runner calculates MD5 checksum for uploads. #729

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

giventocode
Copy link
Contributor

Applies to #728

In this PR:

  • Added a new property to the node task: RuntimeOptions.SetContentMd5OnUpload. When set to true, the runner will compute the MD5 for the files before the block list is committed.
  • This change lays the groundwork for implementing hash validation when downloading data using DRS.
  • By default, the setting is false since calculating the MD5 has a performance impact.

Pending Work (to complete: #728):

  • Expose this option in the TES server as a runtime configuration, with a default value of false.
  • Modify the CoA deployer to include this new configuration option.

@@ -49,7 +49,7 @@ public static string CalculateMd5(string file)
using var md5 = MD5.Create();
using var stream = File.OpenRead(file);
var hash = md5.ComputeHash(stream);
return Convert.ToBase64String(hash);
return BitConverter.ToString(hash).Replace("-", "").ToLowerInvariant();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider Encoding.UTF8.GetString


Logger.LogInformation("MD5 hash calculated for file: {filePath}.", filePath);

return BitConverter.ToString(hash).Replace("-", "").ToLowerInvariant();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@giventocode giventocode merged commit e544138 into main Jun 17, 2024
7 checks passed
@giventocode giventocode deleted the ja/content-md5 branch June 17, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants