Fix https://github.com/dmwm/WMCore/issues/10745

[amaltaro]

@vkuznet Valentin, I just tested your code interactively and it will not fix the problem. See:

In [1]: value = "alan\r\nmalta"
In [2]: isinstance(value, str)
Out[2]: True
In [3]: if isinstance(value, str):
   ...:     value = bytes(value, 'utf-8')

In [4]: value
Out[4]: b'alan\r\nmalta'
In [5]: header = value.split('\r\n')
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-5-9324d14aaf57> in <module>
----> 1 header = value.split('\r\n')

TypeError: a bytes-like object is required, not 'str'

So, you convert the input value to py3 bytes type, but then you try to parse it with a py3 string. Maybe what would need to be fixed is '\r\n' to b'\r\n', or an improved version of that.

[vkuznet]

@amaltaro you're right and I forgot to pass the byte string. Now the PR is updated.

[amaltaro]

Since I had to debug this code in a test agent, here is an implementation that seems to resolve this issue:

        headers = self._response_header.getvalue()
        if isinstance(headers, bytes):
            headers = headers.decode("utf-8")
        for header in headers.split('\r\n'):

and failures inside the client now result in something like:

RestClient.ErrorHandling.RestClientExceptions.HTTPError: HTTP Error 400: Bad Request

Valentin, if you agree, can you please update this PR with the code provided above? Thanks

[vkuznet]

@amaltaro I committed your version. Please double check it.

[amaltaro]

Thanks, Valentin. It looks good to me. @yuyiguo Yuyi, would you be able to run some extra tests/checks with this patch, and if you are happy, merge and release a new version of the python3 dbs3-client?

[yuyiguo]

@amaltaro We are in the middle of transferring DBS Client and PyCurl client into two different repositories PyCurlClient and DBSClient that are move out of DBS repo. These new repos are python 3 and fully tested. The code you have is here in the new repo: https://github.com/dmwm/PycurlClient/blob/main/src/python/RestClient/RequestHandling/HTTPResponse.py#L13 .

However, my summer student did not have the time to make the library working with CMS's special voms . I am going to work on this myself. If it is possible that you could wait for the new code?

[amaltaro]

@yuyiguo Hi Yuyi, sure, if you prefer we could make a PR against the repository as well.

However, my summer student did not have the time to make the library working with CMS's special voms .

I'm afraid I do not follow this command. Is it a fix that must be made on the library dependency to deploy dbs3-client? Or is it a new feature to use voms roles? I'm asking this because we have been using the latest py3 dbs3-client in production for around a month now, and everything is working fine (besides this issue we are fixing here).

[yuyiguo]

@amaltaro Hi Alan,

I did not look into the details of this patch, but the new code should have covered the fixing already?

When the Neil/the summer student built DBS client, he built it outside cms system. So the client did not work with the cms voms. This is not a new feature. When one does "voms-proxy-init", the new client cannot use the certs generated by the command. I need to figure out how to or where to find the CMS library that outside the CMS build system so we can build pypi library of DBS client. I am going to start an email to ask about this and I will add you in the thread.

[amaltaro]

This is the code that should be looking/loading the user certificate: https://github.com/dmwm/PycurlClient/blob/main/src/python/RestClient/AuthHandling/X509Auth.py#L39

Maybe it's just a matter of exporting X509_USER_PROXY before importing dbsApi.

[yuyiguo]

A different issue. The cert was found and put there were errors to use them . We had briefly discussed during Neil's final presentation.

[amaltaro]

Regarding PUT requests, this patch is also required for version 3.16.2000:

--- sw/slc7_amd64_gcc630/cms/py3-dbs3-pycurl-client/3.16.2000/lib/python3.8/site-packages/RestClient/RequestHandling/HTTPRequest.py.orig    2021-10-06 16:49:14.543595641 +0200
+++ sw/slc7_amd64_gcc630/cms/py3-dbs3-pycurl-client/3.16.2000/lib/python3.8/site-packages/RestClient/RequestHandling/HTTPRequest.py 2021-10-06 17:08:59.223963953 +0200
@@ -35,6 +35,7 @@ class HTTPRequest(object):
             ### pycurl will automatically set content-length header using strlen()

         elif method == 'PUT':
+            data = data.encode("utf-8")
             data_fp = BytesIO(data)
             content_length = len(data)
             self._curl_options[pycurl.READFUNCTION] = data_fp.read

pasting it here just so it doesn't get lost in my local changes.

[vkuznet]

@amaltaro , done, please review again.

[amaltaro]

Thanks, Valentin. These changes look good to me!

It just occurred to me that we actually need these changes to be done against the branch py3-4-wm. We could either create another PR pointing against that specific branch, or edit this PR and see whether we can successfully point it to py3-4-wm branch. @vkuznet would you be able to do so?

@yuyiguo if you are back from vacation today, is there any chance that you could merge it and cut a new tag (e.g. 3.16.2001)? With that, we could build a clean RPM. Otherwise, I will try applying a patch at the spec file and keep the same version (but it's somehow ugly).

[vkuznet]

@amaltaro here is PR for py3-4-wm branch: https://github.com/dmwm/DBS/pull/659

[yuyiguo]

@amaltaro I am back and just finished reading/scanning my emails. We are no longer working on DBS client under this repository. I saw you comments in DMWM/DBSClient repository, but I haven't got chance to read details.

Now we are having two repos for DBS client. I am very close to finish up the one in DMWM/DBSClient. You want to use DMWM/DBS for the rpm build for now?

[amaltaro]

@yuyiguo thanks for looking into this. I'm planning to release a final stable WMAgent release before the day is over tomorrow/Tuesday, and even though this fix is not critical, I think it would be an advantage to have this in and avoid unnecessary tracebacks in our logs.

So, if it's not too much extra effort, I would suggest to proceed with this bugfix (actually the one made against branch py3-4-wm) and cut a new tag. Then tomorrow I can make a cmsdist PR and build the new dbs3-client/dbs3-pycurl-client tag together with a new version of WMAgent, to be used in production.

I'm not aware of any other issues, so I think it would be the last client issue to be dealt with from this repository ;)

[yuyiguo]

@amaltaro The ticket says: vkuznet wants to merge 4 commits into dmwm:master from vkuznet:exp-fix But you want to merge it to py3-4-wm. Can you confirm ?

[amaltaro]

@yuyiguo Yuyi, it does not hurt to merge this PR. But the one that we really need is: https://github.com/dmwm/DBS/pull/659

which has the python3 migration developments done by Todor. AFAIK this master branch is not yet python3 compatible.

Once PR 659 is merged, then I would need you to cut a new tag pointing to HEAD of branch py3-4-wm, such that those changes are available in the upcoming tag. PS.: I will be on slack for another 10 or 15min, in case you have any questions it might be quicker to talk over there. Thanks!

[yuyiguo]

695 merged.

[yuyiguo]

@amaltaro new tag 3.16.2001 created. Let me know if you need anything else for tomorrow.

[amaltaro]

@yuyiguo thank you very much, Yuyi!

[amaltaro]

Given that these changes are not required to the master branch (required changes were already merged in the python3 py3-4-wm branch), and that it looks like DBSClient and PycurlClient repositories already have these fixes in, shall we close this PR?

[vkuznet]

fine with me, closing