Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dependency on setuptools #3221

Merged
merged 1 commit into from
Sep 2, 2021
Merged

Remove dependency on setuptools #3221

merged 1 commit into from
Sep 2, 2021

Conversation

ssbarnea
Copy link
Member

Fixes: #3219

@ssbarnea ssbarnea added the bug label Aug 31, 2021
@ssbarnea ssbarnea force-pushed the rm/setuptools branch 2 times, most recently from c3f5e3b to 603fdd8 Compare August 31, 2021 13:09
@ssbarnea ssbarnea marked this pull request as ready for review August 31, 2021 13:26
setup.cfg Outdated
@@ -73,14 +73,15 @@ install_requires =
cookiecutter >= 1.7.3 # dependency issues in older versions
dataclasses; python_version<"3.7"
enrich >= 1.2.5
importlib-metadata<2; python_version<="3.7"
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't this listed among constraints?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is because pip-tools bugs/limitations. I am considering removing the constraints soon unless someone has time to split them python version. Even if we split them per python versions, we will also need one for linux and non linux.

Fixing the deps/constraints is not as easy as it seams. Happy to discuss that but is outside the scope of that PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

This appears to cause problems when trying to install ansible-core + molecule + an openstack component (ex. os-net-config) via constraints when installing under centos8 (e.g. 3.6). importlib-metadata 4.8 works, what is the specific reason to lock it to 2?

@@ -25,8 +25,8 @@
__metaclass__ = type

try:
import pkg_resources
import importlib_metadata
Copy link
Member

@webknjaz webknjaz Aug 31, 2021

Choose a reason for hiding this comment

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

This shouldn't be a part of try-except because it's clearly listed as a dependency.

Suggested change
import importlib_metadata

Copy link
Member Author

Choose a reason for hiding this comment

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

I do not fully remember all the reasons why we had this approach but I think that it was something related to running unittest without installing the package. I will try to remove and see what happens.


__version__ = pkg_resources.get_distribution("molecule").version
__version__ = importlib_metadata.version("molecule") # type: ignore
except Exception:
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that with importlib-metadata we should be having a catch-all handler. Let's do

Suggested change
except Exception:
except importlib_metadata.PackageNotFoundError:


__version__ = pkg_resources.get_distribution("molecule").version
__version__ = importlib_metadata.version("molecule") # type: ignore
Copy link
Member

Choose a reason for hiding this comment

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

Plz use specific ignores rather than ignoring everything.

Suggested change
__version__ = importlib_metadata.version("molecule") # type: ignore
__version__ = importlib_metadata.version("molecule") # type: ignore[a-specific-problem-id]

constraints.txt Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove runtime dependency on setuptools
3 participants