Skip to content

[v7r2] Functional DIRAC client for Python 3#4731

Closed
chrisburr wants to merge 53 commits into
DIRACGrid:integrationfrom
chrisburr:python3-client
Closed

[v7r2] Functional DIRAC client for Python 3#4731
chrisburr wants to merge 53 commits into
DIRACGrid:integrationfrom
chrisburr:python3-client

Conversation

@chrisburr

@chrisburr chrisburr commented Aug 28, 2020

Copy link
Copy Markdown
Member

With these changes the certification hackathon tests pass for a client installed using the environment-py3.yaml file that is used for the unit tests. Next I'll work on getting DIRACOS working so we can use dirac-install.py as well.

As this builds on #4726 which hasn't been merged, see here for the diff chrisburr/DIRAC@run-py3-pytest...python3-client. As usual for these changes, reviewing individual commits might be easier that the entire PR.

The biggest change is chrisburr@fc9c3c7#diff-243fb299d5af97760799bfe30f97e051 where I add a forceBytes=False keyword argument to the recieveData part of RPC calls. This is needed to make it possible to say that the data being received is supposed to be bytes instead of a str as most of DIRAC's use of str is actually ASCII text. This option is only actually needed in a couple of places (downloading the compressed CS and sending files) and the only alternatives I can think are much worse:

  • Make a backwards incompatible change to DEncode
  • Modify everything to use unicode instead of str so Python 2 behaves like Python 3

EDIT with another idea: The forceBytes=True parameter is only needed when receiving data for FileHelper.receiveData and the CS getCompressedDataIfNewer. This ends up being three lines:

An alternative would be to add two new methods on the server side getCompressedBase64DataIfNewer/receiveBase64Data and deprecate the existing methods.

BEGINRELEASENOTES

*Python 3
NEW: Client installation is now functional with Python 3

ENDRELEASENOTES

chrisburr and others added 30 commits August 16, 2020 23:09
Co-authored-by: fstagni <federico.stagni@cern.ch>
@chaen

chaen commented Aug 28, 2020

Copy link
Copy Markdown
Contributor

Without looking at chrisburr@fc9c3c7#diff-243fb299d5af97760799bfe30f97e051 in depth, I'd like to propose an alternative, which we would benefit anyway with HTTPS: stop sending bytes from the CS. We anyway must use base64 in #4677 since we ship data with JSON. And the compression would anyway happen directly in the server, so it would not cost much.
It would be a less dangerous fix, and prepare for the future.
Another option is to add another encoe/decode function couples specifically for bytes, but I am not really fund of that, since again, JSON will eliminate that option.

Just to add, there was always the consensus that the actual client (like ConfigurationClient, FileCatalogClient, etc) knows what type is expected, so that is a convenient location where we could do the cast.

@chrisburr

Copy link
Copy Markdown
Member Author

I'd like to propose an alternative, which we would benefit anyway with HTTPS: stop sending bytes from the CS.

That would break using clients that use the previous DIRAC version wouldn't it? I agree that it should be done, but I think it would be simpler to make it so that HTTPS clients always use base64 and DISET continues with the status quo.

It would be a less dangerous fix

I don't think there is any danger with the solution I currently have as it does nothing on Python 2. It just allows calling functions to say "I expect bytes to come out of this function" which only has an effect in Python 3 and once DISET it removed the option will disappear with it.

@chaen

chaen commented Aug 28, 2020

Copy link
Copy Markdown
Contributor

a try base64.decode/except in the next patch release would solve it.

I don't think there is any danger with the solution I currently have

I do think there is :-D As I said, if that is really something we want to keep, then we should add it in the specific client (in that case, only the CS client), which is much less invasive. It's maybe not the nicest, but I really don't like the idea of mangling in the serialization and the Transport classes, especially now that we want to quickly get out of it.

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.

4 participants