Handle module.file == None #1042

qezz · 2024-03-15T12:28:28Z

It's been discovered that some modules have the __file__ property being set as None. An example of a such module is backports.

Providing None to os.path.abspath() causes an exception: TypeError: expected str, bytes or os.PathLike object, not NoneType

Related to

The Mitogen master process was unable to serve u'ansible_mitogen.target'. on ansible core 2.13 #946

Has been used in the Nix environment:

Python 3.11.8
Ansible 2.13.13
Mitogen 0.3.4 (with this patch)

qezz · 2024-03-15T13:41:42Z

Looking at the errors at one of the failing workflows (https://dev.azure.com/mitogen-hq/mitogen/_build/results?buildId=671&view=logs&j=38fb768a-91bc-52fb-9300-923df8625209&t=0744b325-ce4a-569b-7cdc-1da0e44989ad&l=25)

ERROR: FAIL could not package project - v = InvocationError('/Library/Frameworks/Python.framework/Versions/3.12/bin/python setup.py sdist --formats=zip --dist-dir /Users/runner/work/1/s/.tox/dist', 1)

Not sure it's related to the change.

Also https://dev.azure.com/mitogen-hq/mitogen/_build/results?buildId=671&view=logs&j=93c724b5-725e-5662-44a0-1d07d9df84a7&t=e0f2306a-4d86-5692-a85d-93c8590e39ad

======================================================================
ERROR: setUpClass (su_test.SuTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vsts/work/1/s/tests/testlib.py", line 625, in setUpClass
    cls.dockerized_ssh = DockerizedSshDaemon(**daemon_args)
  File "/home/vsts/work/1/s/tests/testlib.py", line 553, in __init__
    self.start_container()
  File "/home/vsts/work/1/s/tests/testlib.py", line 533, in start_container
    self._get_container_port()
  File "/home/vsts/work/1/s/tests/testlib.py", line 510, in _get_container_port
    self.port = int(bport)
ValueError: invalid literal for int() with base 10: ':]:32768'

qezz · 2024-03-15T14:25:09Z

By the way, the base branch for it is stable, please let me know if it should go to master. I can rebase.

moreati · 2024-04-02T12:34:51Z

By the way, the base branch for it is stable, please let me know if it should go to master. I can rebase.

It should be master. I've updated the PR, please rebase.

It's been discovered that some modules have the `__file__` property being set as `None`. An example of a such module is `backports`. Providing `None` to `os.path.abspath()` causes an exception: `TypeError: expected str, bytes or os.PathLike object, not NoneType` Related to mitogen-hq#946

qezz · 2024-04-09T10:33:14Z

@moreati rebased onto master.

Some CI jobs are still failing, but I don't have enough knowledge to say if it's expected or not.
Please let me know if I should try reproducing it locally.

moreati · 2024-04-09T10:51:21Z

I've just asked Azure Pipelines to rerun the failed jobs. It's probably a case of flaky tests, they're taking 1 or 2 reruns to get 100% green. Thought I had a ticket for it ...

In general one can trigger rerun of all Azure Pipelines jobs in a github project by adding a comment with "/AzurePipelines r-u-n", without the dashes. https://learn.microsoft.com/en-us/azure/devops/pipelines/repos/github?view=azure-devops&tabs=yaml#comment-triggers

moreati

Are you sure this code change is still doing anything? The third argument to getattr(..., ..., '') already provides '' as a default. E.g.

>>> getattr(None, 'does_not_exist', 'default value')
'default value'

qezz · 2024-04-09T11:31:52Z

The __file__ attribute exists there, but it's None

Similar to something like

class X:
    def __init__(self):
        self.attr = None

x = X()
a = getattr(x, 'attr', 'default value')
print(a)
# prints None

Then, if not handled, the following happens

modpath = os.path.abspath(None)

and causes the error

>>> import os
>>> os.path.abspath(None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<frozen posixpath>", line 399, in abspath
TypeError: expected str, bytes or os.PathLike object, not NoneType

moreati · 2024-04-09T12:01:23Z

Added https://github.com/mitogen-hq/mitogen/wiki/import-and-importlib-notes and mentioned this.

moreati · 2024-04-09T12:08:47Z

Hunting for other cases that might need fixing

mitogen.master.DefectivePython3xMainMethod.find() looks okay
mitogen.master.DefectivePython3xMainMethod.find()looks okay
mitogen.core looks okay
mitogen.compat.pkgutil is close to retirement/removal

moreati

Finally this needs

An entry in docs/changelog.rst
Rebasing against current master (I'll hold off merging other PRs for a day or two)

Optionally

Add yourself to docs/contributers.rst

moreati · 2024-04-09T12:17:02Z

mitogen/master.py

This comment is more misleading than helpful. Better to remove it.

moreati · 2024-04-09T12:18:33Z

mitogen/master.py

    if module is None:
        return False

    # six installs crap with no __file__


This comment is now more mislaeading than helpful, please remove it.

moreati · 2024-04-09T12:23:15Z

mitogen/master.py

    # six installs crap with no __file__
-    modpath = os.path.abspath(getattr(module, '__file__', ''))
+    path = getattr(module, '__file__', '')
+    if path is None:


I would like to have an accompanying unit test for this case. If you'd like to add one I recommend doing so in tests.module_finder_test.IsStdlibNameTest, and adding the package that caused this in the wild as a test dependency in tests/requirements.txt.

Otherwise I'm happy to add one.

Adding the package might be tricky, since it seems to be a part of Python distribution, though I'm not 100% sure.
The package was backports.

Will try to reproduce on different environments

backports.* (e.g. https://pypi.org/project/backports.textwrap/) are all thrid party distributions on PyPI. They aren't part of the stdlib.

backports is a namespace package they all declare. The project on PyPI called backports (https://pypi.org/project/backports/) is a placeholder only. I don't think it is meant to be installed, and it doesn't contain an Python code.

So I recommend you use a small backports.<your choice>, or if you don't have a preference use backports.textwrap.

Ah, I see, thanks for clarification 👍

qezz mentioned this pull request Mar 15, 2024

The Mitogen master process was unable to serve u'ansible_mitogen.target'. on ansible core 2.13 #946

Open

moreati changed the title ~~Handle missing module file~~ Handle missing module __file__ attribute Mar 19, 2024

moreati changed the base branch from stable to master April 2, 2024 12:33

qezz force-pushed the 0.3.4-handle-missing-module-file branch from 20fe0fb to 591e542 Compare April 8, 2024 20:30

moreati reviewed Apr 9, 2024

View reviewed changes

moreati changed the title ~~Handle missing module __file__ attribute~~ Handle module.__file__ == None Apr 9, 2024

moreati requested changes Apr 9, 2024

View reviewed changes

Handle module.__file__ == None #1042

Are you sure you want to change the base?

Handle module.__file__ == None #1042

Uh oh!

Conversation

qezz commented Mar 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qezz commented Mar 15, 2024

Uh oh!

qezz commented Mar 15, 2024

Uh oh!

moreati commented Apr 2, 2024

Uh oh!

qezz commented Apr 9, 2024

Uh oh!

moreati commented Apr 9, 2024

Uh oh!

moreati left a comment

Choose a reason for hiding this comment

Uh oh!

qezz commented Apr 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

moreati commented Apr 9, 2024

Uh oh!

moreati commented Apr 9, 2024

Uh oh!

moreati left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

moreati Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

moreati Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

moreati Apr 9, 2024

Choose a reason for hiding this comment

Uh oh!

qezz Apr 10, 2024

Choose a reason for hiding this comment

Uh oh!

moreati Apr 10, 2024

Choose a reason for hiding this comment

Uh oh!

qezz Apr 10, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Handle module.file == None #1042

Handle module.file == None #1042

qezz commented Mar 15, 2024 •

edited

Loading

qezz commented Apr 9, 2024 •

edited

Loading

moreati left a comment •

edited

Loading