Skip to content

Conversation

@ogenstad
Copy link
Contributor

@ogenstad ogenstad commented Dec 20, 2024

This PR fixes an issue that can occur if a Path object is passed to the importer instead of an str, mypy failed to see these issues.

To illustrate the problem that might occur we can create a local file:

cat /tmp/test_module.py
print("successfully imported")

Then in a separate location we run:

import sys
from pathlib import Path

import importlib

path = Path("/tmp")

sys.path.append(path)

importlib.import_module("test_module")

It fails with:

>>> importlib.import_module("test_module")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>

  File "/nix/store/w2sjj4z4bxbh67yw38pqzx290bw3pra8-python3-3.12.5/lib/python3.12/importlib/__init__.py", line 90, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1387, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1324, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'test_module'
>>>

If we instead use:

import sys
import importlib

path = "/tmp"

sys.path.append(path)

importlib.import_module("test_module")

The operation is successful:

>>> importlib.import_module("test_module")
successfully imported
<module 'test_module' from '/tmp/test_module.py'>

Later we should rewrite these functions to instead expect Path objects but ensure that we add them as strings to sys.path.

@codecov
Copy link

codecov bot commented Dec 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

@@           Coverage Diff            @@
##           develop     #181   +/-   ##
========================================
  Coverage    66.16%   66.17%           
========================================
  Files           76       76           
  Lines         7046     7047    +1     
  Branches      1392     1392           
========================================
+ Hits          4662     4663    +1     
  Misses        1998     1998           
  Partials       386      386           
Flag Coverage Δ
python-3.10 44.98% <100.00%> (+<0.01%) ⬆️
python-3.11 44.98% <100.00%> (+<0.01%) ⬆️
python-3.12 44.98% <100.00%> (+<0.01%) ⬆️
python-3.13 44.98% <100.00%> (+<0.01%) ⬆️
python-3.9 44.05% <0.00%> (-0.01%) ⬇️
python-filler-3.12 23.98% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/_importer.py 74.28% <100.00%> (+0.75%) ⬆️

@ogenstad ogenstad marked this pull request as ready for review December 20, 2024 09:42
@ogenstad ogenstad requested a review from a team December 20, 2024 09:42
Copy link
Contributor

@dgarros dgarros left a comment

Choose a reason for hiding this comment

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

All good

@ogenstad ogenstad merged commit 0dac001 into develop Dec 20, 2024
13 checks passed
@ogenstad ogenstad deleted the pog-sys-path branch December 20, 2024 10:15
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.

3 participants