- 
                Notifications
    
You must be signed in to change notification settings  - Fork 557
 
Feat: Add SHA-256 hashing option #988
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
Feat: Add SHA-256 hashing option #988
Conversation
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.
Thank you very much @mdambski, the PR looks great, but I think we can make it simpler as the hashing in kb is not that critical.
Also, I think when we allow the users to configure the caching there is no need to set the default dynamically.
please have a look at my comments and let me know your thoughts, It could be that I've missed the actual purpose. Thanks!
          Documentation preview | 
    
| 
           Hi @Pouyanpi, all change requests are addressed. Please let me know if we can move forward 🙏  | 
    
| 
           Thank you @mdambski you just need to remove the unused config folder  | 
    
| 
           @Pouyanpi the config was removed  | 
    
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.
Thanks, looks good to me.
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.
Tests are failing for Python 3.9 (test_utils.py)
@mdambski would you please have a look?
| 
           @Pouyanpi the tests are fixed 👍  | 
    
| 
           @mdambski Thank you very much. I'll merge this PR later as it is decided to be part of 0.13.0 release. Probably Python 3.12 also needs to wait for 0.13.0, I'll let you know.  | 
    
Description
This change set introduces the
sha256hashing option as an alternative tomd5. By default, the library will usemd5unless this hashing algorithm is unavailable (which would effectively break the library for the user).Rationale
At the moment, the library uses
md5hashing by default and does not allow the option to choose an alternative.In some distributions of Python that are compliant with FIPS Regulation,
md5will either be missing or will raise an error when used. These concerns are brought up in the Python docs related to hash algorithms.In such environments, using a library that relies solely on
md5forces users to either abandon usage of the library or create and maintain their own fork with the needed patches. Both options are far from optimal.cc: @Pouyanpi
Related Issue(s)
Checklist