Skip to content

Conversation

@ThomasLiennard
Copy link

Add new digest_algorithm option to WarcWriter and RecordBuilder to set digest hash algorithm to something other than sha1.

Add new digest_algorithm option to WarcWriter and RecordBuilder to set digest hash algorithm.
@codecov
Copy link

codecov bot commented Nov 3, 2020

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.42%. Comparing base (aa702cb) to head (ec6d33d).
⚠️ Report is 22 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #119   +/-   ##
=======================================
  Coverage   98.42%   98.42%           
=======================================
  Files          18       18           
  Lines        1650     1655    +5     
  Branches      265      266    +1     
=======================================
+ Hits         1624     1629    +5     
  Misses          6        6           
  Partials       20       20           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Mr0grog
Copy link

Mr0grog commented Aug 22, 2023

This would be lovely to have! Given the notes about encoding in the annotated WARC 1.1 spec, it might be nice if it selected a different encoding (or let you specify an encoding) depending on the algorithm (e.g. base-32 encoding a sha-256 digest might get you a technically invalid header value, so base16 is better for that algorithm).

@wumpus
Copy link
Collaborator

wumpus commented Aug 28, 2023

Apologies for being so picky after such a long time, but here are some comments:

  • pullreqs should be made against the "develop" branch
  • the documentation needs to mention this new feature
  • this feature needs to work with all ways that warcio creates warcs, including capture_http()
  • what do you expect to happen with an invalid digest_algorithm name? You should document and test that so that the new interface will remain stable
  • you need to test that warcio check is happy with this digest (very likely, but still test it)
  • you have added some end-of-line whitespace; look at "git diff" to see it

@wumpus
Copy link
Collaborator

wumpus commented Aug 28, 2023

(If you are no longer interested in this PR I'm willing to fix these things)

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.

3 participants