Skip to content

Conversation

@saintstack
Copy link
Contributor

@saintstack saintstack commented Jun 4, 2025

Rockylinux9 is the os that runs our agents (rhel9 and main/default).

Should be no functional changes because of diffs in the below.

joshua_model.py was barfing on call to getchildren removed in 3.9 triggered by attempts at changing xml processing adding optional save of test output.

AttributeError: 'xml.etree.ElementTree.Element' object has no attribute 'getchildren'
0x1000024d7b9be62470000 1 joshua-rhel9-agent-250603151750-76-ns984 6635832187356503052 '<Test><JoshuaMessage Severity="10" Message="value_in_blob" BlobKey="6635832187356503052" BlobVersion="2" /></Test>\n'
Traceback (most recent call last):
  File "/usr/local/lib64/python3.9/site-packages/joshua/joshua_model.py", line 954, in tail_results
    msg = unwrap_message(text)
  File "/usr/local/lib64/python3.9/site-packages/joshua/joshua_model.py", line 186, in unwrap_message
    return root.getchildren()[0].attrib

Most of the below changes came via

pip install pyupgrade
pyupgrade --py39-plus .
find . -name "*.py" -exec pyupgrade --py39-plus {} +

On these sorts of changes:

@@ -116,11 +116,11 @@ def start_ensemble(
properties["timeout"] = timeout

   if fail_fast > 0 and not no_fail_fast:
  •    print("Note: Ensemble will complete after {} failed results.".format(fail_fast))
    
  •    print(f"Note: Ensemble will complete after {fail_fast} failed results.")
     properties["fail_fast"] = fail_fast
    

Replaces str.format() with an f-string for a print() statement. Modern, idiomtic python.

Nothing wrong w/ using set in the below but pyupgrade thinks the change more 'direct':

 if not yes and not dryrun:
     response = input("Do you want to delete these ensembles [y/n]? ")
  •    if response.strip().lower() not in set(["y", "yes"]):
    
  •    if response.strip().lower() not in {"y", "yes"}:
           print("Negative response received. Not performing deletion.")
           return
    

On the below change:

-class TimeoutFuture(object):
+class TimeoutFuture:
def init(self, timeout):
self.cb_list = []
self.timer = threading.Timer(timeout, self._do_on_ready)

In Python 3, classes implicitly inherit from object if no other base class is specified. So, (object) is redundant.

On the below, 'r' is default:

       if env.get('HOSTNAME', False) and os.path.isfile(k8s_namespace_file):
           pod_name = env['HOSTNAME']
  •        with open(k8s_namespace_file, 'r') as f:
    
  •        with open(k8s_namespace_file) as f:
               namespace = f.read()
    

Below is main change. Element.getchildren() was deprecated in earlier Python versions and removed in Python 3.9.

def unwrap_message(text):
root = ET.fromstring(text)

  • return root.getchildren()[0].attrib
  • return next(iter(root)).attrib

On the blow, PEP 585 - Type Hinting Generics In Standard Collections: This PEP, implemented starting in Python 3.9, allows the direct use of built-in collection types (like list, dict, tuple, set, etc.) as generic types in type annotations.

-def list_and_watch_active_ensembles(tr) -> Tuple[List[str], fdb.Future]:
+def list_and_watch_active_ensembles(tr) -> tuple[list[str], fdb.Future]:
return _list_and_watch_ensembles(tr, dir_active, dir_active_changes)

On the below, in Python 3 (specifically starting from Python 3.3), IOError became an alias for OSError. This means that IOError is essentially the same as OSError. OSError is now the canonical name for this broad category of OS-related errors (which includes I/O errors).

  • except IOError:
  • except OSError:
    # This is not our process, so we can't open the file.
    return dict()

On the below, PEP 8 Compliance: The unittest module in Python's standard library has adopted snake_case method names (like assertEqual) as the standard, following PEP 8 guidelines.

 def test_mark_env(self):
     env = mark_environment(dict())
  •    self.assertEquals(os.getpid(), int(env[VAR_NAME]))
    
  •    self.assertEqual(os.getpid(), int(env[VAR_NAME]))
    

@saintstack
Copy link
Contributor Author

I'm trying to figure out how to test.

…linux9 and

the os that runs our agents.

Should be no functional difference in the changes made below.

joshua_model.py was barfing on call to getchildren removed in 3.9
triggered by attempts at changing xml processing.

Most of the below changes came via

   pip install pyupgrade
   pyupgrade --py39-plus .
   find . -name "*.py" -exec pyupgrade --py39-plus {} +

On changes like the below change:

  diff --git a/joshua/joshua.py b/joshua/joshua.py
  index 8065be3..659821d 100755
  --- a/joshua/joshua.py
  +++ b/joshua/joshua.py
  @@ -41,7 +41,7 @@ def get_username():
   def format_ensemble(e, props):
       return "  %-50s %s" % (
           e,
  -        " ".join("{}={}".format(k, v) for k, v in sorted(props.items())),
  +        " ".join(f"{k}={v}" for k, v in sorted(props.items())),
       )

On these sorts of changes:

  @@ -116,11 +116,11 @@ def start_ensemble(
           properties["timeout"] = timeout

       if fail_fast > 0 and not no_fail_fast:
  -        print("Note: Ensemble will complete after {} failed results.".format(fail_fast))
  +        print(f"Note: Ensemble will complete after {fail_fast} failed results.")
         properties["fail_fast"] = fail_fast

Replaces str.format() with an f-string for a print() statement. Modern,
idiomtic python.

Nothing wrong w/ using set in the below but pyupgrade thinks the change
more 'direct':

     if not yes and not dryrun:
         response = input("Do you want to delete these ensembles [y/n]? ")
-        if response.strip().lower() not in set(["y", "yes"]):
+        if response.strip().lower() not in {"y", "yes"}:
             print("Negative response received. Not performing deletion.")
             return

On the below change:

  -class TimeoutFuture(object):
  +class TimeoutFuture:
       def __init__(self, timeout):
           self.cb_list = []
           self.timer = threading.Timer(timeout, self._do_on_ready)

In Python 3, classes implicitly inherit from object if no other base class is specified. So, (object) is redundant.

On the below, 'r' is default:

           if env.get('HOSTNAME', False) and os.path.isfile(k8s_namespace_file):
               pod_name = env['HOSTNAME']
  -            with open(k8s_namespace_file, 'r') as f:
  +            with open(k8s_namespace_file) as f:
                   namespace = f.read()

Below is main change. Element.getchildren() was deprecated in earlier
Python versions and removed in Python 3.9.

 def unwrap_message(text):
     root = ET.fromstring(text)
-    return root.getchildren()[0].attrib
+    return next(iter(root)).attrib

On the blow, PEP 585 - Type Hinting Generics In Standard Collections: This PEP,
implemented starting in Python 3.9, allows the direct use of built-in collection
types (like list, dict, tuple, set, etc.) as generic types in type annotations.

  -def list_and_watch_active_ensembles(tr) -> Tuple[List[str], fdb.Future]:
  +def list_and_watch_active_ensembles(tr) -> tuple[list[str], fdb.Future]:
       return _list_and_watch_ensembles(tr, dir_active, dir_active_changes)

On the below, in Python 3 (specifically starting from Python 3.3), IOError
became an alias for OSError. This means that IOError is essentially the same
as OSError. OSError is now the canonical name for this broad category of
OS-related errors (which includes I/O errors).

-    except IOError:
+    except OSError:
         # This is not our process, so we can't open the file.
         return dict()

On the below, PEP 8 Compliance: The unittest module in Python's standard
library has adopted snake_case method names (like assertEqual) as the
standard, following PEP 8 guidelines.

     def test_mark_env(self):
         env = mark_environment(dict())
-        self.assertEquals(os.getpid(), int(env[VAR_NAME]))
+        self.assertEqual(os.getpid(), int(env[VAR_NAME]))
@saintstack saintstack changed the title Make this code run on python3.9, the default on rockylinux9 and Make this code run on python3.9, the default on rockylinux9 Jun 4, 2025
@saintstack
Copy link
Contributor Author

Hmm... This looks like it deployed. When cicd built the image, it became the 'latest' and then on the next agent run, it picked up these changes. Seems to be working fine.

@saintstack saintstack requested a review from spraza June 4, 2025 23:43
@saintstack saintstack changed the title Make this code run on python3.9, the default on rockylinux9 Make joshua run on python3.9, the default on rockylinux9 Jun 5, 2025
Copy link
Member

@johscheuer johscheuer left a comment

Choose a reason for hiding this comment

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

Is there a reason to choose Python 3.9 and not a newer Python version? 3.9 will be EOL by October 2025 (https://devguide.python.org/versions/).

@spraza
Copy link
Contributor

spraza commented Jun 5, 2025

Code LGTM. Johannes' comment about python version makes sense.

@saintstack
Copy link
Contributor Author

Reason for 3.9 is that this is the version that ships w/ our docker base image os, rockylinux9. I suggest we commit this for now and look at newer python -- or purging python altogether -- separately.

@xis19
Copy link
Contributor

xis19 commented Jun 5, 2025

Reason for 3.9 is that this is the version that ships w/ our docker base image os, rockylinux9. I suggest we commit this for now and look at newer python -- or purging python altogether -- separately.

@saintstack's comment is indeed the reason we need to stick with Python 3.9. Our internal RHEL9 image is also using Python3.9 as its default interpreter. How sad our fdb python binding is still python2 compatible.

@saintstack saintstack merged commit d470277 into FoundationDB:main Jun 5, 2025
2 checks passed
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