Skip to content

[v7r2] Run client integration tests for Python 3#4732

Merged
atsareg merged 70 commits into
DIRACGrid:integrationfrom
chrisburr:python3-client-integration
Oct 22, 2020
Merged

[v7r2] Run client integration tests for Python 3#4732
atsareg merged 70 commits into
DIRACGrid:integrationfrom
chrisburr:python3-client-integration

Conversation

@chrisburr

Copy link
Copy Markdown
Member

The main thing this PR does is crudely add support for passing --pythonVersion=3 to ./dirac-install.py. When this happens it gets the latest conda based installer from https://github.com/chrisburr/DIRACOS2.

As this builds on #4726 which hasn't been merged, see here for the diff chrisburr/DIRAC@python3-client...chrisburr:python3-client-integration.

BEGINRELEASENOTES

*Core
NEW: Support installing Python 3 clients using by passing --pythonVersion=3 to ./dirac-install.py

ENDRELEASENOTES

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

Copy link
Copy Markdown
Member Author

I think this is now ready to be merged. The last few commits do the following:

  • to build upon the HTTPS PR
  • always treat data from X509 certificates as bytes
  • remove the force_bytes parameter that I had added in [v7r2] Functional DIRAC client for Python 3 #4731 in favour of using Unicode surrogate escapes. This allows the interfaces to be kept the same while still passing arbitrary bytes in strings. This workaround is only needed while DEncode still exists.

I haven't tidied up the dirac-install script as I think this should be done in a separate PR to make it easier to review (#4775).

If people want to see the reviews of the earlier commits, they can be found in #4718, #4726 and #4731.

@chrisburr chrisburr marked this pull request as ready for review October 21, 2020 11:07

:param ReleaseConfig releaseConfig: The ReleaseConfig object for configuring the installation
"""
url = "https://github.com/chrisburr/DIRACOS2/releases/latest/download/DIRACOS-Linux-x86_64.sh"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just reminder for changing this link.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be done as part of #4775. I haven't set up DIRACGrid/DIRACOS2 yet as I'd like to have tags for the tornado packages rather than building from a branch.

@fstagni

fstagni commented Oct 21, 2020

Copy link
Copy Markdown
Contributor

IMHO this can be merged, at least to avoid conflicts.

I would merge this BEFORE any other PRs targeting the integration branch. @atsareg

fstagni
fstagni previously approved these changes Oct 21, 2020
Comment thread Core/Utilities/Grid.py Outdated

@andresailer andresailer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't answer the other one until I submit?

impData[0].close()
except ImportError as excp:
if str(excp).find("No module named %s" % modName[0]) == 0:
if "No module named" in str(excp) and modName[0] in str(excp):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if "No module named" in str(excp) and modName[0] in str(excp):
if ("No module named %s" % modName[0]) in str(excp):

Otherwise it isn't completely the same test, if the exception message is "No module named Something"+modName[0]
(Not sure if parenthesis needed or operator precedence is already enough

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional as the message is different.

Python 2:

In [1]: import abcs
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-1-9ec4e562403a> in <module>()
----> 1 import abcs

ImportError: No module named abcs

Python 3:

In [1]: import abcs
---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
<ipython-input-1-9ec4e562403a> in <module>
----> 1 import abcs

ModuleNotFoundError: No module named 'abcs'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, that is annoying.

# import re  # already done in the module
if re.findall("No module named '?%s'?" % modName[0], str(excp)):

In [2]: s1 = "No module named abcs"

In [3]: s2=  "No module named 'abcs'"

In [4]: re.findall("No module named '?abcs'?", s1)
Out[4]: ['No module named abcs']

In [5]: re.findall("No module named '?abcs'?", s2)
Out[5]: ["No module named 'abcs'"]

In [6]: re.findall("No module named '?abcs'?", 'No module named abcsfoo')
Out[6]: ['No module named abcs'] # even the old test was broken in this regard, similar python3

In [7]: re.findall("No module named '?abcs'?", 'No module named fooabcs')
Out[7]: []

Comment thread DataManagementSystem/Client/FileCatalogClientCLI.py Outdated
Comment thread Interfaces/API/Dirac.py

COMPONENT_NAME = 'DiracAPI'

try:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not if six.PY2 here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its hard to explain but feels more natural to use try...except here. I want to check if file is defined rather than use my knowledge about the Python versions involved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are other cases where try..except are used, but then six wasn't imported, so I felt I had to ask.
It isn't a problem, as long as no one does something like file = 'myfile.txt' somewhere above (unlikely, but 🤷).

andresailer
andresailer previously approved these changes Oct 21, 2020

@andresailer andresailer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the minor comments posted

Co-authored-by: Andre Sailer <andre.philippe.sailer@cern.ch>
@chrisburr chrisburr dismissed stale reviews from andresailer and fstagni via 2d54b12 October 22, 2020 07:52
Co-authored-by: Andre Sailer <andre.philippe.sailer@cern.ch>
@atsareg atsareg merged commit c04db6d into DIRACGrid:integration Oct 22, 2020
@chrisburr chrisburr deleted the python3-client-integration branch October 22, 2020 15:49
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.

5 participants