Page MenuHomeSoftware Heritage

Add missing __init__.py so find_packages keep finding sql modules
ClosedPublic

Authored by ardumont on Dec 12 2022, 2:11 PM.

Diff Detail

Repository
rDSCH Scheduling utilities
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build is green

Patch application report for D8952 (id=32256)

Rebasing onto 8e125f1d98...

Current branch diff-target is up to date.
Changes applied before test
commit 66e582b6625f528c3d9567ebc1b1546ec7039bf9
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Dec 12 14:05:46 2022 +0100

    setup.py: Use find-package-namespace instead of find-package
    
    The latter discards folder directories without __init__.py while the former
    will scan them nonetheless.
    
    Another option would be to keep find-package and add the missing
    __init__.py...
    
    In any case, without this, this complains in the debian build otherwise [1]
    [2].
    
    [1]
11:52:47      ############################
11:52:47      # Package would be ignored #
11:52:47      ############################
11:52:47      Python recognizes 'swh.scheduler.sql' as an importable package,
11:52:47      but it is not listed in the `packages` configuration of setuptools.
11:52:47
11:52:47      'swh.scheduler.sql' has been automatically added to the distribution only
11:52:47      because it may contain data files, but this behavior is likely to change
11:52:47      in future versions of setuptools (and therefore is considered deprecated).
11:52:47
11:52:47      Please make sure that 'swh.scheduler.sql' is included as a package by using
11:52:47      the `packages` configuration field or the proper discovery methods
11:52:47      (for example by using `find_namespace_packages(...)`/`find_namespace:`
11:52:47      instead of `find_packages(...)`/`find:`).
11:52:47
11:52:47      You can read more about "package discovery" and "data files" on setuptools
11:52:47      documentation page.
11:52:47
11:52:47
11:52:47  !!
11:52:47
11:52:47    check.warn(importable)
11:52:47  /usr/lib/python3/dist-packages/setuptools/command/build_py.py:202: SetuptoolsDeprecationWarning:     Installing 'swh.scheduler.sql.upgrades' as data is deprecated, please list it in `packages`.
11:52:47      !!
11:52:47
11:52:47
11:52:47      ############################
11:52:47      # Package would be ignored #
11:52:47      ############################
11:52:47      Python recognizes 'swh.scheduler.sql.upgrades' as an importable package,
11:52:47      but it is not listed in the `packages` configuration of setuptools.
11:52:47
11:52:47      'swh.scheduler.sql.upgrades' has been automatically added to the distribution only
11:52:47      because it may contain data files, but this behavior is likely to change
11:52:47      in future versions of setuptools (and therefore is considered deprecated).
11:52:47
11:52:47      Please make sure that 'swh.scheduler.sql.upgrades' is included as a package by using
11:52:47      the `packages` configuration field or the proper discovery methods
11:52:47      (for example by using `find_namespace_packages(...)`/`find_namespace:`
11:52:47      instead of `find_packages(...)`/`find:`).
11:52:47
11:52:47      You can read more about "package discovery" and "data files" on setuptools
11:52:47      documentation page.

```

[1h2] https://jenkins.softwareheritage.org/view/swh-debian%20(draft)/job/debian/job/packages/job/DSCH/job/gbp-buildpackage/182/console
See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/589/ for more details.

Thanks for looking at this (fwiw, this warning is pretty innocuous for now, and is not what prevents the Debian build; the issue seems to be in pytest).

The commit message needs editing (it should read setup.py: use find_namespace_packages instead of find_packages).

I don't think inlining the full warning in the commit message provides much value, over just saying something along the lines of "The discovery of implicit namespace packages (under regular namespaces) is being deprecated by setuptools".

I'm not quite sure what the implications of find_namespace_packages are. If we're going that route, I think we should explicitly only include namespace packages named "swh.*" (so, adding include=["swh.*"] to the find_namespace_packages call). Instead of fishing for these implications, I'd be tempted to consider getting rid of setuptools in favor of a simpler python packaging helper (e.g. flit) instead of chasing after more setuptools deprecations/behavior changes (see also the workarounds we have to use to keep the editable mode setup in docker).

All in all, maybe just adding the missing __init__.py files would be cheaper for now?

Thanks for looking at this (fwiw, this warning is pretty innocuous for now,
and is not what prevents the Debian build; the issue seems to be in pytest).

yes, totally.

The commit message needs editing (it should read `setup.py: use
find_namespace_packages instead of find_packages`).

ack

I don't think inlining the full warning in the commit message provides much
value, over just saying something along the lines of "The discovery of
implicit namespace packages (under regular namespaces) is being deprecated
by setuptools".

right. It was mostly for reviewer to not have to follow the link.

I'm not quite sure what the implications of find_namespace_packages
are. If we're going that route, I think we should explicitly only include
namespace packages named "swh.*" (so, adding include=["swh.*"] to the
find_namespace_packages call). Instead of fishing for these implications,
I'd be tempted to consider getting rid of setuptools in favor of a simpler
python packaging helper (e.g. flit) instead of chasing after more setuptools
deprecations/behavior changes (see also the workarounds we have to use to
keep the editable mode setup in docker).

ah, did not know about other caveats we already worked around.

All in all, maybe just adding the missing __init__.py files would be
cheaper for now?

yes, all in all with what you added, i think so too. I'll adapt.

Adapt according to discussion

Build has FAILED

Patch application report for D8952 (id=32261)

Rebasing onto 8e125f1d98...

Current branch diff-target is up to date.
Changes applied before test
commit 5f063274ef2f283411d712b71ee2d9eb93a9573c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Dec 12 14:05:46 2022 +0100

    Add missing __init__.py so find_packages keep finding sql modules
    
    Otherwise, at some point, this will get discarded as per the debian build
    warning [1]
    
    [1] https://jenkins.softwareheritage.org/view/swh-debian%20(draft)/job/debian/job/packages/job/DSCH/job/gbp-buildpackage/182/console

Link to build: https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/590/
See console output for more information: https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/590/console

ardumont retitled this revision from setup.py: Use find-package-namespace instead of find-package to Add missing __init__.py so find_packages keep finding sql modules.Dec 12 2022, 4:01 PM
ardumont edited the summary of this revision. (Show Details)
ardumont edited the summary of this revision. (Show Details)

Fix

Build is green

Patch application report for D8952 (id=32262)

Rebasing onto 8e125f1d98...

Current branch diff-target is up to date.
Changes applied before test
commit 77ac1d39aea6aaa6cd3024ca326591b4974af4e2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Dec 12 14:05:46 2022 +0100

    Add missing __init__.py so find_packages keep finding sql modules
    
    Otherwise, at some point, this will get discarded as per the debian build
    warning [1]
    
    [1] https://jenkins.softwareheritage.org/view/swh-debian%20(draft)/job/debian/job/packages/job/DSCH/job/gbp-buildpackage/182/console

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/591/ for more details.

This revision is now accepted and ready to land.Dec 15 2022, 9:55 AM

Build is green

Patch application report for D8952 (id=32376)

Rebasing onto d521ab706e...

Current branch diff-target is up to date.
Changes applied before test
commit fccf9442fcddae509164f3c9be8055854d344700
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Dec 12 14:05:46 2022 +0100

    Add missing __init__.py so find_packages keep finding sql modules
    
    Otherwise, at some point, this will get discarded as per the debian build
    warning [1]
    
    [1] https://jenkins.softwareheritage.org/view/swh-debian%20(draft)/job/debian/job/packages/job/DSCH/job/gbp-buildpackage/182/console

See https://jenkins.softwareheritage.org/job/DSCH/job/tests-on-diff/592/ for more details.