Issue/drop pkg resources - first part #7990

hlloreda · 2024-08-21T11:56:18Z

Description

First part that migrates the use of the deprecated library: pkg_resources. This part preserves the venv caching mechanism that allows us to speed up the test suite. The second part should migrate this mechanism

Related to #6668

Self Check:

Strike through any lines that are not applicable (~~line~~) then check the box

Attached issue to pull request
Changelog entry
Type annotations are present
Code is clear and sufficiently documented
No (preventable) type errors (check using make mypy or make mypy-diff)
Sufficient test cases (reproduces the bug/tests the requested feature)
Correct, in line with design
End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )
If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see test-fixes for more info)

# Conflicts: # src/inmanta/agent/agent.py # src/inmanta/agent/executor.py

hlloreda · 2024-08-21T13:32:59Z

src/inmanta/main.py

@@ -182,7 +182,7 @@ def get_table(header: list[str], rows: list[list[str]], data_type: Optional[list
    return table.draw()


-@with_plugins(iter_entry_points("inmanta.cli_plugins"))
+@with_plugins(importlib_metadata.entry_points(group="inmanta.cli_plugins"))


mypy is raising a warning here:

src/inmanta/main.py:185: error: Argument 1 to "with_plugins" has incompatible type "EntryPoints"; expected "Generator[EntryPoint, None, None]" [arg-type]

I don't know how should I fix this

Suggested change

@with_plugins(importlib_metadata.entry_points(group="inmanta.cli_plugins"))

@with_plugins(iter(importlib_metadata.entry_points(group="inmanta.cli_plugins")))

like so?

this doesn't seem to be maintained I think it is good to figure out why it is here at all (I suspect LSM).

We may want to vendor it or find a fork if we need it.

click-contrib/click-plugins#37

We are now experiencing this error:

src/inmanta/main.py:185: error: Argument 1 to "iter" has incompatible type "EntryPoints"; expected "SupportsIter[Generator[EntryPoint, None, None]]" [arg-type] src/inmanta/main.py:185: note: Following member(s) of "EntryPoints" have conflicts: src/inmanta/main.py:185: note: Expected: src/inmanta/main.py:185: note: def __iter__(self) -> Generator[EntryPoint, None, None] src/inmanta/main.py:185: note: Got: src/inmanta/main.py:185: note: def __iter__(self) -> Iterator[Any]

yes, click-plugin is just outdated, perhaps we can vendor it?

wouterdb

is there a migration guide that was followed? (I don't know if it exists, didn't find it either)

src/inmanta/env.py

wouterdb · 2024-08-28T09:32:31Z

src/inmanta/env.py

+class SafeRequirement(Requirement):
+    def __init__(self, requirement_string: str) -> None:
+        # Packaging Requirement is not able to parse requirements with comment. Therefore, we need to remove the `comment` part
+        drop_comment = requirement_string.split("#")[0]


How does this handle the inputs

"" "#" " # " "#this is a comment"

src/inmanta/env.py

wouterdb · 2024-08-28T09:35:09Z

src/inmanta/env.py

                # set as a marker. The line below makes sure that the "extra" marker matches. The marker is not set by
                # `Distribution.requires()` when the package is installed in editable mode, but setting it always doesn't make
                # the marker evaluation fail.
                environment_marker_evaluation = {"extra": contained_in_extra} if contained_in_extra else None
                if r.marker and not r.marker.evaluate(environment=environment_marker_evaluation):
                    # The marker of the requirement doesn't apply on this environment
                    continue
-                if r.key not in installed_packages or str(installed_packages[r.key]) not in r:
+                if (
+                    r.name.lower() not in installed_packages


why is the lower added?

wouterdb · 2024-08-28T11:58:59Z

src/inmanta/env.py

@@ -234,7 +253,7 @@ def get_packages_in_working_set(cls, inmanta_modules_only: bool = False) -> dict
        :param inmanta_modules_only: Only return inmanta modules from the working set
        """
        return {
-            dist_info.key: version.Version(dist_info.version)
+            utils.canonicalize_name(dist_info.key): version.Version(dist_info.version)


We may want to write a comment or adr about name Vs canonicalize_name. What should be used where.

wouterdb · 2024-08-28T12:01:39Z

src/inmanta/main.py

@@ -182,7 +182,7 @@ def get_table(header: list[str], rows: list[list[str]], data_type: Optional[list
    return table.draw()


-@with_plugins(iter_entry_points("inmanta.cli_plugins"))
+@with_plugins(importlib_metadata.entry_points(group="inmanta.cli_plugins"))


Suggested change

@with_plugins(importlib_metadata.entry_points(group="inmanta.cli_plugins"))

@with_plugins(iter(importlib_metadata.entry_points(group="inmanta.cli_plugins")))

like so?

wouterdb · 2024-08-28T12:04:37Z

src/inmanta/main.py

@@ -182,7 +182,7 @@ def get_table(header: list[str], rows: list[list[str]], data_type: Optional[list
    return table.draw()


-@with_plugins(iter_entry_points("inmanta.cli_plugins"))
+@with_plugins(importlib_metadata.entry_points(group="inmanta.cli_plugins"))


this doesn't seem to be maintained I think it is good to figure out why it is here at all (I suspect LSM).

We may want to vendor it or find a fork if we need it.

wouterdb · 2024-08-28T12:05:15Z

src/inmanta/main.py

@@ -182,7 +182,7 @@ def get_table(header: list[str], rows: list[list[str]], data_type: Optional[list
    return table.draw()


-@with_plugins(iter_entry_points("inmanta.cli_plugins"))
+@with_plugins(importlib_metadata.entry_points(group="inmanta.cli_plugins"))


click-contrib/click-plugins#37

src/inmanta/module.py

hlloreda · 2024-09-30T10:49:24Z

src/inmanta/module.py

@@ -107,7 +105,7 @@ def __init__(self, requirement: packaging.requirements.Requirement) -> None:
                f"Problematic case: {str(requirement)}"
            )
        self._requirement: inmanta.util.CanonicalRequirement = inmanta.util.parse_requirement(str(requirement))
-        self._project_name = re.sub('[^A-Za-z0-9.]+', '-', requirement.name)
+        self._project_name = re.sub("[^A-Za-z0-9.]+", "-", requirement.name)


~~Maybe it would be better to only receive a string?~~

sanderr

As discussed in person

sanderr

As discussed

src/inmanta/module.py

inmantaci · 2024-10-03T07:26:08Z

Processing this pull request

inmantaci · 2024-10-03T07:26:11Z

Failed to merge changes into iso7 due to merge conflict. Please open a pull request for these branches separately by cherry-picking the commit that was made on the branch master (git cherry-pick d048e72).

inmantaci · 2024-10-03T07:26:14Z

Merged into branches master in d048e72

# Description First part that migrates the use of the deprecated library: pkg_resources. This part preserves the venv caching mechanism that allows us to speed up the test suite. The second part should migrate this mechanism Related to #6668 # Self Check: Strike through any lines that are not applicable (`~~line~~`) then check the box - [ ] Attached issue to pull request - [ ] Changelog entry - [ ] Type annotations are present - [ ] Code is clear and sufficiently documented - [ ] No (preventable) type errors (check using make mypy or make mypy-diff) - [ ] Sufficient test cases (reproduces the bug/tests the requested feature) - [ ] Correct, in line with design - [ ] End user documentation is included or an issue is created for end-user documentation (add ref to issue here: ) - [ ] If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)

inmantaci · 2024-10-03T07:26:15Z

Not closing this pull request due to previously commented issues for some of the destination branches. Please open a separate pull request for those branches by cherry-picking the relevant commit. You can safely close this pull request and delete the source branch.

inmantaci · 2024-10-03T07:26:40Z

This branch was not deleted as it seems to still be in use.

# Description Implementation of #7990 for ISO7 Related to #6668 # Self Check: Strike through any lines that are not applicable (`~~line~~`) then check the box - [ ] Attached issue to pull request - [ ] Changelog entry - [ ] Type annotations are present - [ ] Code is clear and sufficiently documented - [ ] No (preventable) type errors (check using make mypy or make mypy-diff) - [ ] Sufficient test cases (reproduces the bug/tests the requested feature) - [ ] Correct, in line with design - [ ] End user documentation is included or an issue is created for end-user documentation (add ref to issue here: ) - [ ] If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)

hlloreda · 2024-10-03T14:17:27Z

Everything has been merged (ISO7 and master)

hlloreda added 14 commits July 31, 2024 13:57

first commit

37ee4b1

Merge branch 'refs/heads/master' into issue/drop_pkg_resources

7d333fe

wip

021e009

mypy

1779105

Merge branch 'refs/heads/master' into issue/drop_pkg_resources

25893db

refactor

871e18d

lower name

3a5743a

test

a6a6dde

Merge branch 'refs/heads/master' into issue/drop_pkg_resources

c49b37e

# Conflicts: # src/inmanta/agent/agent.py # src/inmanta/agent/executor.py

refactor

a32e383

Merge branch 'master' into issue/drop_pkg_resources

3ea487c

rollback changes

3377996

rollback changes

880d9da

refactor

c5187f8

hlloreda self-assigned this Aug 21, 2024

hlloreda commented Aug 21, 2024

View reviewed changes

hlloreda added 3 commits August 21, 2024 15:38

refactor

a239a06

refactor

06ea0cf

fix broken test

aeb68cb

hlloreda marked this pull request as ready for review August 21, 2024 14:23

hlloreda changed the title ~~Issue/drop pkg resources 2~~ Issue/drop pkg resources - first part Aug 21, 2024

hlloreda requested review from sanderr and wouterdb August 22, 2024 07:42

wouterdb requested changes Aug 28, 2024

View reviewed changes

hlloreda added 6 commits August 29, 2024 10:48

Merge branch 'master' into issue/drop_pkg_resources_2

c5d3246

refactor

87e381a

refactor

ae4915b

Merge branch 'master' into issue/drop_pkg_resources_2

4b8e2ad

refactor

0768b2b

add test safe requirement

8d71c66

hlloreda added 3 commits September 30, 2024 11:05

rollback

ba01c4d

Merge branch 'master' into issue/drop_pkg_resources_2

b12c5d2

refactor

24692f8

hlloreda commented Sep 30, 2024

View reviewed changes

hlloreda added 2 commits September 30, 2024 12:54

refactor

dc95c99

refactor to make sure to not share this instance

3e71ebf

hlloreda requested review from wouterdb and sanderr October 1, 2024 06:52

wouterdb approved these changes Oct 1, 2024

View reviewed changes

sanderr requested changes Oct 1, 2024

View reviewed changes

hlloreda added 3 commits October 1, 2024 17:36

Merge branch 'master' into issue/drop_pkg_resources_2

2d79e9e

refactor

d77a95d

remove constraint on setuptools

af785c4

sanderr approved these changes Oct 2, 2024

View reviewed changes

missing revert changes

8e4ae57

sanderr requested changes Oct 2, 2024

View reviewed changes

src/inmanta/module.py Outdated Show resolved Hide resolved

missing revert changes

73d9cf3

sanderr approved these changes Oct 2, 2024

View reviewed changes

hlloreda added the merge-tool-ready This ticket is ready to be merged in label Oct 3, 2024

inmantaci removed the merge-tool-ready This ticket is ready to be merged in label Oct 3, 2024

hlloreda mentioned this pull request Oct 3, 2024

Drop pkg resources iso7 - first part #8152

Closed

9 tasks

hlloreda closed this Oct 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue/drop pkg resources - first part #7990

Issue/drop pkg resources - first part #7990

hlloreda commented Aug 21, 2024 •

edited

Loading

hlloreda Aug 21, 2024

wouterdb Aug 28, 2024

wouterdb Aug 28, 2024

wouterdb Aug 28, 2024

hlloreda Aug 29, 2024

wouterdb Aug 30, 2024

wouterdb left a comment

wouterdb Aug 28, 2024

wouterdb Aug 28, 2024

wouterdb Aug 28, 2024

wouterdb Aug 28, 2024

wouterdb Aug 28, 2024

wouterdb Aug 28, 2024

hlloreda Sep 30, 2024 •

edited

Loading

sanderr left a comment

sanderr left a comment

inmantaci commented Oct 3, 2024

inmantaci commented Oct 3, 2024

inmantaci commented Oct 3, 2024

inmantaci commented Oct 3, 2024

inmantaci commented Oct 3, 2024

hlloreda commented Oct 3, 2024

	@with_plugins(importlib_metadata.entry_points(group="inmanta.cli_plugins"))
	@with_plugins(iter(importlib_metadata.entry_points(group="inmanta.cli_plugins")))

Issue/drop pkg resources - first part #7990

Issue/drop pkg resources - first part #7990

Conversation

hlloreda commented Aug 21, 2024 • edited Loading

Description

Self Check:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wouterdb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hlloreda Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

sanderr left a comment

Choose a reason for hiding this comment

sanderr left a comment

Choose a reason for hiding this comment

inmantaci commented Oct 3, 2024

inmantaci commented Oct 3, 2024

inmantaci commented Oct 3, 2024

inmantaci commented Oct 3, 2024

inmantaci commented Oct 3, 2024

hlloreda commented Oct 3, 2024

hlloreda commented Aug 21, 2024 •

edited

Loading

hlloreda Sep 30, 2024 •

edited

Loading