-
Notifications
You must be signed in to change notification settings - Fork 35
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
Give more helpful encoding exceptions in pack_sim #545
Conversation
The `requests` package is pulled in through the dependency tree. As of requests==2.30.0 it uses urllib3>=2 which is incompatible with the version of OpenSSL found on RGS machines.
Codecov Report
@@ Coverage Diff @@
## master #545 +/- ##
==========================================
+ Coverage 86.17% 86.21% +0.04%
==========================================
Files 49 49
Lines 7025 7046 +21
==========================================
+ Hits 6054 6075 +21
Misses 971 971
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
4ae73c7
to
341f414
Compare
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.
@mferrera very nice 👍
Do we need to check it as well for inspect_file
and _md5checksum
as well?
Also would it be nice/needed to print out the correct encoding to end-user? I see that komodo
has chardet
library install which can guess the correct encoding format or alternatively file -ib
341f414
to
c9818c7
Compare
I don't think we need to check in There are a number of possible improvements that could address possibly-unintentional logic like this in the script but maybe out of scope. I'm not certain most users would gain a lot of actionable insight if we tell them the encoding. My guess is that the vast majority will probably find ISO-8859-1 a bit meaningless to them 😅 but if you think it's a good idea I'm happy to add it |
c9818c7
to
6b59885
Compare
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.
@mferrera looks good to me.
Regarding printing out correct encoding, I’m not sure either. I saw in slack channel that user can change/convert the file encoding by using iconv command and specify which encoding to convert from.
Resolves #537
A specific command to do the conversion could be given but it's probably better not to be responsible if it damages the file.