Improve pylint and mypy linting by not requiring directory structure outside of repository #1

felixfontein · 2025-03-16T12:04:23Z

~~This is currently blocked by pylint-dev/pylint#10278.~~

gotmax23 · 2025-03-16T23:09:30Z

Does the error happen in a clean venv or only when the ansible package that provides ansible_collections in site packages is also installed?

felixfontein · 2025-03-17T05:07:26Z

The error happens in the venv created by nox. There's ansible-core in there, but not ansible. What's worse, sometimes it happens, sometimes it does not.

I do need to use the rsync way to reproduce it though: https://github.com/ansible-community/antsibull-nox/pull/1/files#diff-998a45901c6eeed5c113b8b118eecc0a55676298af65200193c383975fd33fadR372 (or a copy). With the symlinks (that's commented in right now) there are other errors (No name 'dns' in module 'ansible_collections.community').

felixfontein · 2025-03-17T20:21:08Z

After some trial and error I found out that I don't really need the __init__.py in the collection's root, so everything's fine. (It took me some time to notice that the reason that mypy didn't find most of the intentional typing errors I introduced is because they happen in functions without a type signature - and these are ignored 🤦.)

I'm currently polishing the PR and will push something soon.

felixfontein · 2025-03-17T21:23:26Z

Meh, for whatever reason this seems to be hitting pylint-dev/pylint#5832...

felixfontein · 2025-03-17T22:17:24Z

I also created wntrblm/nox#943 since for some reason nox runs the linting with Python 3.11 instead of 3.13.

gotmax23 · 2025-03-18T16:34:05Z

I also created wntrblm/nox#943 since for some reason nox runs the linting with Python 3.11 instead of 3.13.

The way the nox actions selects Python versions is kind of wonky. Might be better to just use setup-python to install Python and then pipx or uv to run nox.

…is used in CI...

felixfontein · 2025-03-18T20:32:08Z

Yep, with Python 3.13 the pylint bug is gone...

src/antsibull_nox/collection.py

src/antsibull_nox/sessions.py

Co-authored-by: Maxwell G <[email protected]>

src/antsibull_nox/collection.py

gotmax23 · 2025-03-18T20:51:31Z

src/antsibull_nox/collection.py

+        for namespace in root.iterdir():
+            try:
+                if namespace.is_dir() or namespace.is_symlink():
+                    for name in root.iterdir():


Is this supposed to iterate through root a second time?

Definitely not ;-)

(And yes, we really need tests...)

src/antsibull_nox/collection.py

gotmax23 · 2025-03-18T20:57:21Z

src/antsibull_nox/collection.py

+    copier.copy(collection.path, path, exclude_root=[".nox", ".tox"])
+
+
+def _copy_collection_rsync_hard_links(collection: CollectionData, path: Path) -> None:


Is this still needed?

Right now no, but I'm thinking about offering this (or a variant of it) as an option in the future.

We could also potentially implement this functionality as a separate Copier in antsibull_fileutils.

Totally, yes.

gotmax23 · 2025-03-18T20:58:34Z

src/antsibull_nox/sessions.py

@@ -52,8 +62,56 @@ def install(session: nox.Session, *args: str, editable: bool = False, **kwargs):
    session.install(*args, "-U", **kwargs)


+@dataclass
+class CollectionSetup:


Could you document what each of these attributes are? It's not immediately clear based on the names.

src/antsibull_nox/sessions.py

gotmax23 · 2025-03-18T21:08:21Z

Another thing, it would be nice if something like this where dependencies are also installed in the flat structure was supported:

gh repo clone ansible-collections/community.internal_test_tools
gh repo clone ansible-collections/community.dns
cd community.dns
nox

felixfontein · 2025-03-18T21:16:18Z

Another thing, it would be nice if something like this where dependencies are also installed in the flat structure was supported:
gh repo clone ansible-collections/community.internal_test_tools
gh repo clone ansible-collections/community.dns
cd community.dns
nox

~~I think your cd is missing a ansible-collections/ ;-)~~ (Edit: misread something.) But yeah, that would be nice indeed, and should be pretty simple to add. (Edit: created #6)

Co-authored-by: Maxwell G <[email protected]>

gotmax23

Other than the exception handling, this looks good to me

gotmax23 · 2025-03-18T22:11:25Z

src/antsibull_nox/collection.py

+                                    name=name.name,
+                                    root=root,
+                                )
+                        except:  # noqa: E722, pylint: disable=bare-except


Suggested change

except: # noqa: E722, pylint: disable=bare-except

except Exception:

at least, otherwise, this will catch KeyboardInterrupt and such. Or should this only catch ValueError because _load_collection_data_from_disk doesn't (directly) raise anything else?

(This also applies to the other callers of _load_collection_from_disk)

It should also catch I/O errors if someone did something strange in that directory structure so that some other call fails.

gotmax23 · 2025-03-18T22:17:42Z

src/antsibull_nox/collection.py

+    copier.copy(collection.path, path, exclude_root=[".nox", ".tox"])
+
+
+def _copy_collection_rsync_hard_links(collection: CollectionData, path: Path) -> None:


We could also potentially implement this functionality as a separate Copier in antsibull_fileutils.

gotmax23 · 2025-03-18T22:22:58Z

I think your cd is missing a ansible-collections/ ;-) But yeah, that would be nice indeed, and should be pretty simple to add.

Doesn't this code allow checking out collections without a parent ansible_collections directory?

felixfontein · 2025-03-19T06:12:41Z

Yes, I only misread something.

felixfontein · 2025-03-21T21:45:33Z

Thanks for reviewing this, @gotmax23!

felixfontein force-pushed the improve-pylint-mypy branch from 200213e to cc41807 Compare March 16, 2025 21:01

felixfontein mentioned this pull request Mar 17, 2025

Allow to exclude top-level files and dirs when copying trees ansible-community/antsibull-fileutils#14

Merged

felixfontein mentioned this pull request Mar 18, 2025

Install nox directly to use the correct Python version #4

Merged

felixfontein added 3 commits March 18, 2025 21:25

Improve pylint and mypy calls.

3b0cf12

Improve behavior so that pylint and mypy work better.

1742a73

Add changelog fragment.

de48ac5

felixfontein force-pushed the improve-pylint-mypy branch from ffb55a2 to db617b5 Compare March 18, 2025 20:26

Try to fix pylint/asteroid bug on Python 3.11, which for some reason …

c882bbe

…is used in CI...

felixfontein force-pushed the improve-pylint-mypy branch from db617b5 to c882bbe Compare March 18, 2025 20:28

Bump antsibull-fileutils requirement to 1.2.0+.

58df7ce

felixfontein marked this pull request as ready for review March 18, 2025 20:41

gotmax23 reviewed Mar 18, 2025

View reviewed changes

src/antsibull_nox/collection.py Outdated Show resolved Hide resolved

felixfontein commented Mar 18, 2025

View reviewed changes

src/antsibull_nox/sessions.py Outdated Show resolved Hide resolved

Add missing return value types.

8a32575

Co-authored-by: Maxwell G <[email protected]>

gotmax23 self-requested a review March 18, 2025 20:52

gotmax23 reviewed Mar 18, 2025

View reviewed changes

felixfontein and others added 2 commits March 18, 2025 22:21

Improve type.

bc0b747

Co-authored-by: Maxwell G <[email protected]>

Improve documentation, fix import, improve type.

851ee17

felixfontein mentioned this pull request Mar 18, 2025

Find dependent collections in ../<namespace>.<name> when CWD is not part of ansible_collections tree #6

Closed

Fix loop.

e5df1de

gotmax23 reviewed Mar 18, 2025

View reviewed changes

Avoid bare excepts.

d9af0e9

gotmax23 approved these changes Mar 20, 2025

View reviewed changes

felixfontein merged commit bbaa662 into ansible-community:main Mar 21, 2025
2 checks passed

felixfontein deleted the improve-pylint-mypy branch March 21, 2025 21:45

		copier.copy(collection.path, path, exclude_root=[".nox", ".tox"])


		def _copy_collection_rsync_hard_links(collection: CollectionData, path: Path) -> None:

	except: # noqa: E722, pylint: disable=bare-except
	except Exception:

Improve pylint and mypy linting by not requiring directory structure outside of repository #1

Improve pylint and mypy linting by not requiring directory structure outside of repository #1

Uh oh!

Conversation

felixfontein commented Mar 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gotmax23 commented Mar 16, 2025

Uh oh!

felixfontein commented Mar 17, 2025

Uh oh!

felixfontein commented Mar 17, 2025

Uh oh!

felixfontein commented Mar 17, 2025

Uh oh!

felixfontein commented Mar 17, 2025

Uh oh!

gotmax23 commented Mar 18, 2025

Uh oh!

felixfontein commented Mar 18, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gotmax23 commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

felixfontein commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gotmax23 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gotmax23 commented Mar 18, 2025

Uh oh!

felixfontein commented Mar 19, 2025

Uh oh!

Uh oh!

felixfontein commented Mar 21, 2025

Uh oh!

Uh oh!

felixfontein commented Mar 16, 2025 •

edited

Loading

gotmax23 commented Mar 18, 2025 •

edited

Loading

felixfontein commented Mar 18, 2025 •

edited

Loading