Sweep the tree for everything conditional on python 2. by albu-diku · Pull Request #425 · ucphhpc/migrid-sync

albu-diku · 2026-01-27T16:38:58Z

The only thing explicitly left out is the walk relted code in fileio which is rather weedy and being tackled in a separate change.

albu-diku · 2026-01-27T16:49:35Z

Have found everything I can for now - remaining lint failures are pre-existing issues in the code that complaints now because the corresponding are only applied to changed files, which removal of the constant has made true here.

jonasbardino · 2026-01-27T18:15:31Z

Have found everything I can for now - remaining lint failures are pre-existing issues in the code that complaints now because the corresponding are only applied to changed files, which removal of the constant has made true here.

Thanks, and yes, we can just point to #338 about the lint errors here, like I did in #424 about the corresponding work on cleaning out the listdir, scandir and walk related leftovers. However, we need to sync on what to cover here and there AFAICT.

jonasbardino · 2026-01-27T18:21:03Z

Well, I suppose we could easily just fix the missing self argument in

migrid-sync/tests/support/wsgisupp.py

Line 81 in e13d7ec

def close():

which is not covered by #338, but I agree that it's not new and probably slipped in before we had the lint action.

************* Module tests.support.wsgisupp
tests/support/wsgisupp.py:81:8: E0211: Method 'close' has no argument (no-method-argument)

albu-diku · 2026-01-28T11:51:21Z

Well, I suppose we could easily just fix the missing self argument in

migrid-sync/tests/support/wsgisupp.py

Line 81 in e13d7ec

def close():

which is not covered by #338, but I agree that it's not new and probably slipped in before we had the lint action.
************* Module tests.support.wsgisupp
tests/support/wsgisupp.py:81:8: E0211: Method 'close' has no argument (no-method-argument)

I've got a stack of other changes to wsgisupp elsewhere, so I'd prefer to add this fix either there or to fix it alongsside a test that exercises whatever path through the code requires this. :)

jonasbardino · 2026-01-28T12:06:00Z

Well, I suppose we could easily just fix the missing self argument in

migrid-sync/tests/support/wsgisupp.py

Line 81 in e13d7ec

def close():

which is not covered by #338, but I agree that it's not new and probably slipped in before we had the lint action.
************* Module tests.support.wsgisupp
tests/support/wsgisupp.py:81:8: E0211: Method 'close' has no argument (no-method-argument)
I've got a stack of other changes to wsgisupp elsewhere, so I'd prefer to add this fix either there or to fix it alongsside a test that exercises whatever path through the code requires this. :)

Fine with me, it was just to point out that not all lint errors here were already registered and covered e.g. by #338 as I initially thought. I'll let you decide if you want to keep track of it in a new wsgisupp branch or make a mock PR like #338.

…h cause noisy errors in annotation as e.g. seen in PR #425 .

The only thing explicitly left out is the walk relted code in fileio which is rather weedy and being tackled in a separate change.

jonasbardino · 2026-02-04T10:43:04Z

mig/shared/compat.py

    return textual_output
-
-
-def NativeStringIO(initial_value=''):


We have only two modules using NativeStringIO, namely

mig/shared/ssh.py (importing from compat.py)

mig/server/grid_sftp.py (importing from base.py
We probably want to either completely eliminate the wrapper in those two cases, or keep the wrapper here in compat.py so that it is completely clear that it only has relevance in compatibility context and can be phased out along with other py2+3 helpers.

jonasbardino · 2026-02-04T10:45:57Z

mig/shared/defaults.py

 MIG_BASE = os.path.realpath(os.path.join(os.path.dirname(__file__), '../..'))
 MIG_ENV = os.getenv('MIG_ENV', 'default')

-# NOTE: python3 switched strings to use unicode by default in contrast to bytes


I think we'll want to preserve a short note about the point of these variables and their use in shared.base to prevent inadvertent future changes.

jonasbardino · 2026-02-04T10:54:03Z

mig/server/grid_events.py

                    # to ensure that all rule files are loaded

-                    for ent in scandir(src_path):
+                    for ent in os.scandir(src_path):


These are obsolete with the merge of #424

jonasbardino · 2026-02-04T10:54:35Z

mig/shared/base.py

+    return [os.fsencode(arg) for arg in argv]


 def NativeStringIO(initial_value=''):


See note below about NativeStringIO

jonasbardino · 2026-02-04T10:55:13Z

mig/shared/ssh.py

-from mig.shared.base import client_id_dir, force_utf8
+from mig.shared.base import client_id_dir, force_utf8, NativeStringIO
 from mig.shared.conf import get_resource_exe, get_configuration_object
-from mig.shared.compat import NativeStringIO


See note about NativeStringIO above

jonasbardino · 2026-02-04T10:57:41Z

mig/shared/url.py

-
-# NOTE: moved to urllib.parse in python3 and are re-exposed with future.
-#       Other modules should import helpers from here for consistency.
-# TODO: handle the unicode returned by python3 and future versions!


Hmm, I suppose this is resolved or just works now but it would be nice to either comment about specifically in the PR / commit log or add a regression test for instead of just silently dropping the TODO.

jonasbardino

The PR is still left marked as draft and the changes to scandir calls no longer apply after merging #424, but I've commented inline and will leave it to you to (eventually) update it before it will be considered for merging @albu-diku.

jonasbardino · 2026-02-04T11:04:29Z

For the record I've adjusted the lint annotation helper to ignore some common expected mypy errors, which we will generally hit unless we add various explicit type annotations in our code base. There's no need to have them show up as loud errors in the changes tab as they did here before the adjustment.

albu-diku mentioned this pull request Jan 27, 2026

Eliminate PY2 legacy from safeinput where it causes lint errors #419

Merged

albu-diku force-pushed the retire/PY2-constant-and-conditionals branch from 28821b6 to e13d7ec Compare January 27, 2026 16:47

jonasbardino added the refactor Non-functional changes to simplify or clean up label Jan 27, 2026

jonasbardino added this to the Eliminate legacy python2 code and references milestone Jan 27, 2026

jonasbardino mentioned this pull request Jan 28, 2026

Retire the python2 workarounds for inefficient listdir, scandir and walk #424

Merged

jonasbardino force-pushed the retire/PY2-constant-and-conditionals branch from e13d7ec to eb81f25 Compare February 4, 2026 09:32

jonasbardino added a commit that referenced this pull request Feb 4, 2026

Disable some mypy type definition checks we so far never use and whic…

a7969fa

…h cause noisy errors in annotation as e.g. seen in PR #425 .

jonasbardino force-pushed the retire/PY2-constant-and-conditionals branch from eb81f25 to 3a23c64 Compare February 4, 2026 10:13

Sweep the tree for everything conditional on python 2.

d0a3a82

The only thing explicitly left out is the walk relted code in fileio which is rather weedy and being tackled in a separate change.

jonasbardino force-pushed the retire/PY2-constant-and-conditionals branch from 3a23c64 to d0a3a82 Compare February 4, 2026 10:19

jonasbardino reviewed Feb 4, 2026

View reviewed changes

jonasbardino requested changes Feb 4, 2026

View reviewed changes

		return [os.fsencode(arg) for arg in argv]


		def NativeStringIO(initial_value=''):

Conversation

albu-diku commented Jan 27, 2026

Uh oh!

albu-diku commented Jan 27, 2026

Uh oh!

jonasbardino commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonasbardino commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

albu-diku commented Jan 28, 2026

Uh oh!

jonasbardino commented Jan 28, 2026

Uh oh!

jonasbardino Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

jonasbardino Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

jonasbardino Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

jonasbardino Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

jonasbardino Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

jonasbardino Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

jonasbardino left a comment

Choose a reason for hiding this comment

Uh oh!

jonasbardino commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jonasbardino commented Jan 27, 2026 •

edited

Loading

jonasbardino commented Jan 27, 2026 •

edited

Loading