Skip to content

[v7r1] Fix use of io.open in BundleDeliveryClient#4728

Merged
atsareg merged 1 commit into
DIRACGrid:rel-v7r1from
chrisburr:fix-bundle-delivery
Sep 14, 2020
Merged

[v7r1] Fix use of io.open in BundleDeliveryClient#4728
atsareg merged 1 commit into
DIRACGrid:rel-v7r1from
chrisburr:fix-bundle-delivery

Conversation

@chrisburr

Copy link
Copy Markdown
Member

In #4643 various uses of io.open where added. In BundleDeliveryClient this was incorrect as the written object was still a str type instead of unicode. The exception is caught by the except BaseException and has the effect of causing the CAs and CRLs to be downloaded every time as the .dab.CAs and .dab.CRLs files are always empty causing the hash to never match with the server version.

As DIRAC isn't going to convert every str object to unicode before migrating to Python 3 it makes sense to stick to open() instead of using io.open().

BEGINRELEASENOTES

*Framework
FIX: Fix hashing of local CA and CRL bundles

ENDRELEASENOTES

@chaen

chaen commented Aug 26, 2020

Copy link
Copy Markdown
Contributor

As DIRAC isn't going to convert every str object to unicode before migrating to Python 3 it makes sense to stick to open() instead of using io.open().

You mean in general or just this specific case ?

I seem to have understand that open and io.open are the same for python 3. So the problem is going to bite us as soon as we will run this with python 3 no ?

@chrisburr

Copy link
Copy Markdown
Member Author

You mean in general or just this specific case ?

Having just seen this specific issue, I think in general. A bunch of file modes are specified incorrectly in DIRAC, presumably to avoid the error I talk about on the next line.

I seem to have understand that open and io.open are the same for python 3. So the problem is going to bite us as soon as we will run this with python 3 no ?

On Python2 io.open(filename, "wt") requires unicode objects to be written to it. io.open(filename, "wb") requires str. The idea being that unicode is like a Python 3 str and a Python 2 str is like an overpowered Python 3 bytes class (as there are implicit decoding to a real string).

As DIRAC mostly assumes everything is ASCII and just uses Python 2 str everywhere it's probably easier to stick to the builtin open instead of converting 99% of DIRAC's str objects to unicode. It'll "just work" for most cases on Python 3, and the handful where it doesn't I'll add encode/decode which is a no-op for a Python 2 str containing ASCII.

@atsareg

atsareg commented Sep 11, 2020

Copy link
Copy Markdown
Contributor

Is there a good (ultimate) guide which brings together all the recipes of how to use all the python3 encode/decode/unicode/... string manipulation madness ?

@chrisburr

Copy link
Copy Markdown
Member Author

Is there a good (ultimate) guide which brings together all the recipes of how to use all the python3 encode/decode/unicode/... string manipulation madness ?

The page I'd suggest giving a read is: https://portingguide.readthedocs.io/en/latest/strings.html

That said, it doesn't really apply as DIRAC is broken for unicode as it uses str almost everywhere for variables which should be unicode. Following the "best practices" of using unicode for objects which we intend to be text will cause us more trouble that it's worth for something which will mostly fix itself once we move to Python 3.

@atsareg atsareg merged commit 0b40379 into DIRACGrid:rel-v7r1 Sep 14, 2020
@chaen

chaen commented Sep 17, 2020

Copy link
Copy Markdown
Contributor

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