fix: Robust resource lookup for editable installs - #6183
Conversation
📝 WalkthroughWalkthroughReplaced pkg_resources usage with importlib.resources.files to locate the package config file in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Dear contributor, |
|
The issue is: After install avocado and avocado-vt with >>> from avocado.core import data_dir
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/var/home/kar/workspace/avocado/avocado/core/__init__.py", line 20, in <module>
from avocado.core.dispatcher import InitDispatcher
File "/var/home/kar/workspace/avocado/avocado/core/dispatcher.py", line 26, in <module>
from avocado.core.enabled_extension_manager import EnabledExtensionManager
File "/var/home/kar/workspace/avocado/avocado/core/enabled_extension_manager.py", line 20, in <module>
from avocado.core.settings import settings
File "/var/home/kar/workspace/avocado/avocado/core/settings.py", line 839, in <module>
settings = Settings() # pylint: disable-msg=invalid-name
^^^^^^^^^^
File "/var/home/kar/workspace/avocado/avocado/core/settings.py", line 379, in __init__
self._prepare_base_dirs()
File "/var/home/kar/workspace/avocado/avocado/core/settings.py", line 419, in _prepare_base_dirs
if resource_exists("avocado", config_pkg_base):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.12/site-packages/pkg_resources/__init__.py", line 1205, in resource_exists
return get_provider(package_or_requirement).has_resource(resource_name)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.12/site-packages/pkg_resources/__init__.py", line 407, in get_provider
return _find_adapter(_provider_factories, loader)(module)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.12/site-packages/pkg_resources/__init__.py", line 1491, in __init__
self.module_path = os.path.dirname(getattr(module, '__file__', ''))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<frozen posixpath>", line 181, in dirname
TypeError: expected str, bytes or os.PathLike object, not NoneType |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
avocado/core/settings.py(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Fedora develop install/uninstall task
- GitHub Check: Fedora selftests
- GitHub Check: Egg task fedora:40
- GitHub Check: Podman spawner with 3rd party runner plugin
- GitHub Check: Egg task ubi:9.2
- GitHub Check: Analyze (python)
- GitHub Check: Code Coverage (3.11)
- GitHub Check: Build Package (wheel/tarball) for Python 3.9
- GitHub Check: Build Package (wheel/tarball) for Python 3.11
- GitHub Check: Build Package (wheel/tarball) for Python 3.8
- GitHub Check: Version task ubi:8.8
- GitHub Check: Windows with Python 3.10
- GitHub Check: Windows with Python 3.11
- GitHub Check: Windows with Python 3.9
- GitHub Check: Version task debian:12.4
- GitHub Check: macOS with Python 3.11
- GitHub Check: Windows with Python 3.13
- GitHub Check: Version task ubuntu:22.04
- GitHub Check: Smokecheck on Linux with Python 3.10
- GitHub Check: Static checks
🔇 Additional comments (2)
avocado/core/settings.py (2)
45-45: Excellent modernization: Replace deprecated pkg_resources with importlib.resourcesThis change modernizes the resource handling by using the recommended
importlib.resources.filesAPI instead of the deprecatedpkg_resources. This is a best practice update that improves compatibility and robustness.
417-420: Robust resource lookup implementation for editable installsThe implementation correctly uses the modern
importlib.resources.filesAPI to construct the resource path and check for file existence. This approach is more robust than the previouspkg_resourcesmethods, especially for editable installs where the module's__file__attribute might be None.The logic is equivalent to the previous implementation:
- Constructs the path using the
/operator (modern pathlib-style)- Checks existence with
is_file()instead ofresource_exists- Converts to string path when the resource exists
|
Hello @richtja @harvey0100, can you help review this PR? The issue only happens with BTW, GH actions have some failures, but consider Python 3.8 and Ubuntu 20.04 are all out of support, so are we plan to remove them from the CI check? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6183 +/- ##
==========================================
+ Coverage 68.33% 68.34% +0.01%
==========================================
Files 205 205
Lines 22413 22413
==========================================
+ Hits 15315 15318 +3
+ Misses 7098 7095 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @PaulYuuu PR looks good to me, but first of all want to have a discussion with Jan and Cleber about this and review the current issues happening with our CI and decide what we will do. |
Thank you @harvey0100, I agree with you, and let me list some info here. Python3.8
Ubuntu 20.04
Ubi8.8? Not sure if we still want it. BTW, this PR only removed pkg_resources from settings.py, for others files which use pkg_resources, we should remove them or pin the setuptools <81, see:
|
richtja
left a comment
There was a problem hiding this comment.
Hi @PaulYuuu, IIUIC these changes will need deprecation of python 3.8, unfortunately I am not sure if we are prepared for stopping the support of python 3.8. We will need a brother discussion with avocado-vt and tp maintainers to find out if we can do it.
Sure, this only happens in |
Reference: avocado-framework#6183 Signed-off-by: Jan Richter <jarichte@redhat.com>
Reference: avocado-framework#6183 Signed-off-by: Jan Richter <jarichte@redhat.com>
This commit resolves a TypeError that occurred when avocado and avocado-vt was installed in editable mode. Previously, pkg_resources would fail to correctly resolve resource paths (like avocado.conf) because the module's __file__ attribute could be None. To fix this, we've switched to importlib.resources.files for locating package data. This modern API is more reliable and correctly handles resource lookups across various installation methods, including editable installs. Signed-off-by: Yihuang Yu <yihyu@redhat.com>
4c0745e to
f04bef6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (48)
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: Podman spawner with 3rd party runner plugin
- GitHub Check: Egg task ubuntu:20.04
- GitHub Check: Fedora develop install/uninstall task
- GitHub Check: Fedora selftests
- GitHub Check: Version task fedora:40
- GitHub Check: Build Package (wheel/tarball) for Python 3.11
- GitHub Check: Experimental checks
- GitHub Check: Build Package (wheel/tarball) for Python 3.13
- GitHub Check: Build Package (wheel/tarball) for Python 3.9
- GitHub Check: Build Package (wheel/tarball) for Python 3.12
- GitHub Check: Build Package (wheel/tarball) for Python 3.10
- GitHub Check: Egg task ubi:9.2
- GitHub Check: Version task debian:12.4
- GitHub Check: Version task ubi:8.8
- GitHub Check: Smokecheck on Linux with Python 3.10
- GitHub Check: Smokecheck on Linux with Python 3.11
- GitHub Check: Version task ubuntu:22.04
- GitHub Check: Static checks
- GitHub Check: macOS with Python 3.11
- GitHub Check: Code Coverage (3.11)
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
152-152: Verify the runner label ‘windows-2025’.GitHub-hosted Windows runners are typically windows-latest/windows-2022. Please confirm that windows-2025 is a valid, available label in your org. If not, switch to a supported label to avoid job failures.
If not valid, update to:
- runs-on: windows-2025 + runs-on: windows-2022Or:
- runs-on: windows-2025 + runs-on: windows-latest
| steps: | ||
| - name: Install Python dependencies | ||
| run: apt update && apt -y install python3 python3-setuptools ca-certificates | ||
| run: apt update && apt -y install python3.9 python3-setuptools ca-certificates |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Installing python3.9 doesn’t switch the default interpreter (python3 still points to 3.8 on ubuntu:20.04).
As written, downstream steps called by the composite action likely continue to use Python 3.8. If the goal is to run with 3.9 (to ensure importlib.resources.files availability), explicitly switch python3 to 3.9 and ensure setuptools/pip exist for 3.9 inside the container.
Apply this diff to set python3 -> 3.9 and bootstrap pip/setuptools:
- run: apt update && apt -y install python3.9 python3-setuptools ca-certificates
+ run: |
+ apt update
+ apt -y install python3.9 python3.9-venv python3.9-distutils ca-certificates
+ update-alternatives --install /usr/bin/python3 python3 /usr/bin/python3.9 2
+ update-alternatives --set python3 /usr/bin/python3.9
+ python3 -m ensurepip --upgrade || true
+ python3 -m pip install -U "setuptools<80" wheelEnsure default python3 is v3.9 in CI
On ubuntu-20.04, installing python3.9 does not switch the /usr/bin/python3 symlink (it remains Python 3.8). Downstream steps will continue to use 3.8 unless you explicitly change it. You have two options:
• Use the built-in setup step (recommended):
- uses: actions/setup-python@v4
with:
python-version: 3.9• Or manually install and switch via apt:
- run: apt update && apt -y install python3.9 python3-setuptools ca-certificates
+ run: |
+ apt update
+ apt install -y python3.9 python3.9-distutils python3.9-venv ca-certificates
+ update-alternatives --install /usr/bin/python3 python3 /usr/bin/python3.9 2
+ update-alternatives --set python3 /usr/bin/python3.9
+ python3 -m ensurepip --upgrade || true
+ python3 -m pip install --upgrade pip setuptools wheel🤖 Prompt for AI Agents
In .github/workflows/ci.yml around line 354, the workflow installs python3.9 via
apt but does not update /usr/bin/python3, so CI still uses Python 3.8; fix by
either replacing the apt install with the actions/setup-python@v4 step
specifying python-version: 3.9 (recommended), or if you must install via apt,
after installing python3.9 run the appropriate update-alternatives commands to
register python3.9 and set it as the default /usr/bin/python3 (and ensure pip
for that python is available).
| steps: | ||
| - name: Install Python dependencies | ||
| run: apt update && apt -y install python3 python3-setuptools | ||
| run: apt update && apt -y install python3.9 python3-setuptools |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
Same issue for egg_task_ubuntu_20: python3 will remain 3.8 unless explicitly switched.
Building eggs via the composite action will likely invoke python3 (3.8). Ensure the job actually uses 3.9 and has setuptools/pip for that interpreter.
Apply this diff:
- run: apt update && apt -y install python3.9 python3-setuptools
+ run: |
+ apt update
+ apt -y install python3.9 python3.9-venv python3.9-distutils
+ update-alternatives --install /usr/bin/python3 python3 /usr/bin/python3.9 2
+ update-alternatives --set python3 /usr/bin/python3.9
+ python3 -m ensurepip --upgrade || true
+ python3 -m pip install -U "setuptools<80" wheelEnsure Python 3.9 Is the Default and Has pip/setuptools Installed
The python3-setuptools package targets the system’s default Python 3 (3.8 on Ubuntu 20.04), so simply installing python3.9 + python3-setuptools won’t:
- Make
python3point to 3.9 - Provide
pip/setuptoolsfor the 3.9 interpreter
Apply this update in .github/workflows/ci.yml (around line 456):
- run: apt update && apt -y install python3.9 python3-setuptools
+ run: |
+ apt update
+ apt -y install python3.9 python3.9-venv python3.9-distutils
+ update-alternatives --install /usr/bin/python3 python3 /usr/bin/python3.9 2
+ update-alternatives --set python3 /usr/bin/python3.9
+ python3 -m ensurepip --upgrade || true
+ python3 -m pip install -U "setuptools<80" wheelThis ensures:
python3invokes Python 3.9- A
venvmodule and distutils are available pipandsetuptoolsare bootstrapped for 3.9
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| run: apt update && apt -y install python3.9 python3-setuptools | |
| run: | | |
| apt update | |
| apt -y install python3.9 python3.9-venv python3.9-distutils | |
| update-alternatives --install /usr/bin/python3 python3 /usr/bin/python3.9 2 | |
| update-alternatives --set python3 /usr/bin/python3.9 | |
| python3 -m ensurepip --upgrade || true | |
| python3 -m pip install -U "setuptools<80" wheel |
🤖 Prompt for AI Agents
In .github/workflows/ci.yml around line 456, the workflow installs python3.9
plus python3-setuptools which only targets the system default Python (3.8 on
Ubuntu 20.04) and therefore won't make python3 point to 3.9 or provide
pip/setuptools for the 3.9 interpreter; replace that step with installing
python3.9, python3.9-venv and python3.9-distutils, then use update-alternatives
to register and set /usr/bin/python3 to python3.9, and finally bootstrap
pip/setuptools for 3.9 by running python3.9 -m ensurepip --upgrade (or python3.9
-m pip install --upgrade pip setuptools) so the workflow invokes the correct
interpreter and has pip/venv/distutils available.
7244cd1 to
74bf027
Compare
|
Hello @richtja @harvey0100 , as you can see, ubuntu:20.04 still using python3.8. And if I install python3.9 instead of python3, it does not have the entry Considering ubuntu 20.04 is EOL, so is possible remove it from our GitHub Actions? |
Hi @PaulYuuu, yes you are right, I have missed that in #6197 sorry about that. Could you please introduce a commit here with drop of I can open a separated PR about this change, but IMO that would block you even longer and won't be efficient. |
Sure, I will update later. |
Drop ubuntu 20.04 egg and version tasks, as it's EOL. Instead, introduce new ubuntu 24.04 tasks to cover new ubuntu version. Signed-off-by: Yihuang Yu <yihyu@redhat.com>
74bf027 to
a2ea8b8
Compare
Reference: avocado-framework#6183 Signed-off-by: Jan Richter <jarichte@redhat.com>
richtja
left a comment
There was a problem hiding this comment.
Hi @PaulYuuu, thanks for the updates it LGTM.
Reference: avocado-framework#6183 Signed-off-by: Jan Richter <jarichte@redhat.com>
This commit resolves a TypeError that occurred when avocado and avocado-vt was installed in editable mode. Previously, pkg_resources would fail to correctly resolve resource paths (like avocado.conf) because the module's file attribute could be None.
To fix this, we've switched to importlib.resources.files for locating package data. This modern API is more reliable and correctly handles resource lookups across various installation methods, including editable installs.
Summary by CodeRabbit