Py3 dbs3 client - setup changes

[todor-ivanov]

READY!

Those are changes needed to fix only the rpm build process for the py3-dbs3-client.spec and py3-dbs3-pycurl-client.spec from the cmsdist repository

[amaltaro]

Nice catch Todor.

@yuyiguo Yuyi, do you think we could skip building the DBS documentation for a while? Maybe we could create another GH issue to track it in the near future? Please let us know what your thoughts are.

[yuyiguo]

@todor-ivanov @amaltaro Please go head w/o building DBS doc. I will create GH issue to reminder us for putting back the automatically generated docs back.

[yuyiguo]

FYI https://github.com/dmwm/DBS/issues/650

[todor-ivanov]

Thanks @yuyiguo! BTW There are going to be some more changes to be added to the current PR. I will write here again shortly.

[todor-ivanov]

@vkuznet @yuyiguo @amaltaro please take a look at the jsonwrapper suggested here - Still alot of testing to be done

[vkuznet]

@todor-ivanov , few things:

• the standard json library only provide load, loads, dump and dumps, the encode and decode methods only belongs to cjson. Therefore, if you indent to replace cjson.encode and cjson.decode in DBS code it is better to replace them with jsonwrapper.loads and jsonwrapper.dumps methods to stay consistent with APIs of standard json library
• there is no need to have other json's implementation if you never use them. The jsonwrapper provides examples how to add another json implementation, and if necessary it can be done. Therefore you may simplify jsonwrapper to eliminate other json's implementation
• the PR provides not related to json transition changes and it is better to leave them aside, but I know that DBS codebase is not ideal. For instance, there is a mix of spaces and tabs in a code which makes it very hard to read and deal with in vim. But it is another story.

[todor-ivanov]

Thnks @vkuznet !

the PR provides not related to json transition changes and it is better to leave them aside

The initial intention was to fix everything related to py3 migration in dbs3-client, but not only the cjson dependency

For instance, there is a mix of spaces and tabs in a code which makes it very hard to read and deal with in vim.

Not just hard to read. It actually prevents the code from running at all in ipython3, so those changes are indeed needed for the rest of the testing

When it comes to supporting not needed libraries I agree with you - I am going to simplify the wrapper, but I am about to leave ujson in because I found it a quite good substitute both interface related and performance related.

[yuyiguo]

For instance, there is a mix of spaces and tabs in a code which makes it very hard to read and deal with in vim.

In the good old days, everyone used vi and defined their own indentations, some used spaces and some used tabs. Various people touched DBS code so these two are mixed in the code. What I did was that I use modern tools , such as MS VSC to format the entire file when I work on it.

[yuyiguo]

I agreed with Valentin's suggestion on using standard json library with loads/dumps instead of cjson's encode/decode.

[vkuznet]

@todor-ivanov I just noticed that you put code into DBS/Client area while cjson is used both in DBS Server and Client. If you'll keep it in Client then DBS Server code will depend on client unless you'll duplicate the code. Instead, you should make an independent package and put it on pypi. Doing this way it will allow code (DBS Client and Server as well as WMCore) depend on it. The wrapper can be and should be (in my opinion) packaged independently from DBS codebase. But I'll leave a decision about it to WMCore group and @yuyiguo

[todor-ivanov]

Hi @vkuznet @yuyiguo Thanks for the feedback. With my 5th commit to the current PR [1] I tried to simplify the jsonwrapper coul you please take a look and tell me if it is satisfactory enough, so I may continue with the rest of the code modifications for migrating the dbsclient to py3. As of the following:

• the standard json library only provide load, loads, dump and dumps, the encode and decode methods only belongs to cjson. Therefore, if you indent to replace cjson.encode and cjson.decode in DBS code it is better to replace them with jsonwrapper.loads and jsonwrapper.dumps methods to stay consistent with APIs of standard json library

I agree it would be more close to the standard if we do so, but actually in the code there are several places [2] where it actually relies on the cjson.{Encode,Decode}Error exceptions. And because of that following Valentin's initial approach seemed reasonable to me - i.e. wrapping the standard .loads && .dumps calls with the .encode && .decode methods from the jsonwrapper, so that the exceptions coming from the actual json.loads && json.dumps calls can be enveloped (from a single place) with the common set of exceptions defined in the jsonwrrapper and then handled in the code the same way they were before. This way we avoid the need to juggle with exceptions in the dbsclient code. Otherwise in the DBS clinet code we will have to handle two types of exceptions - those from cjson and those from json.

[1] https://github.com/dmwm/DBS/pull/649/commits/f859582832963c751453836cb65f163a5bbea103

[2] https://github.com/dmwm/DBS/blob/14df8bbe8ee8f874fe423399b18afef911fe78c7/Client/src/python/dbs/apis/dbsClient.py#L210

[todor-ivanov]

Hi @vkuznet

I just noticed that you put code into DBS/Client area while cjson is used both in DBS Server and Client ...

I agree that the higher in the tree we put the jsonwrapper the better. My initial intention was not to solve all DBS python3 issues, and that's why I restrained myself only to the code base related to DBSClient, with the intend at some near future, the people responsible for DBS code to take care of positioning it at the right place at the right moment. @amaltaro @yuyiguo please let me know what you think.

[vkuznet]

Regarding exceptions. There are two pieces required. One the jsonwrapper should wrap them such that exception will come from a single method, like load/loads or dump/dumps. Basically you need to catch cjson exception within jsonwrapper and then through again like json module does.

The second part is to modify DBS code itself to catch just common exception which will come either from json module or from jsonwrapper. This will solve issue with different json implementations.

[yuyiguo]

@todor-ivanov

Restraining your current work on DBS client is what I think you may do now. I am thinking we should separate DBS client into a different package/repo from DBS server, but that is later. So we will reviste DBS client later, for now get it to work with py3 first. Then, WM can move forward quickly. Regarding the decode/encode, it is idea to follow py3 json as discussed, but we can live with the cjson style for now.

[amaltaro]

Todor and I had a chat yesterday and we seem to be aligned with what Yuyi said above. We should focus on the minimum amount of changes in dbs3-client and dbs3-pycurl-client to have it compatible with python3, especially because the rest of the DBS code future is also uncertain at this stage.

A few options for those minimum changes that come to my mind are:

• provide this jsonwrapper to the dbs3-client/dbs3-pycurl-client packages only, leaving all the rest of DBS code untouched;
• simply replace cjson by json in the dbs3-client/dbs3-pycurl-client code; and keep it in a separate branch.
• if more change are needed to these client packages, we could consider as well a 2to3 conversion of the client code from python2 to python3; and keep it in a separate branch for now.

Just my 2cents.

[klannon]

I fully agree. To reinforce what you said, for this purpose, there's no need to try to keep compatibility with Py2 (as we did for WMCore). We just need something (on a separate branch, if necessary) that works ASAP so that we can move forward with validation of WMCore under Py3.

[yuyiguo]

I did both "future" and "2to3" on entire DBS code. The changes were not much in terms of types of changes. I think it is safe to move DBS client to py3 and put it in a separate branch.

[todor-ivanov]

As discussed in a meeting we had between me, @amaltaro and @mapellidario this PR was split in two separate PRs:

• The current PR provides the minimal changes in setup.py.
• The new PR provides all the python3 related changes.

p.s. This PR is created against master and can safely be merged there - this will not break the py2 version of dbs-client