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

Should we add a DeprecationWarning to pkg_resources.declare_namespace? #3434

Merged
merged 4 commits into from Feb 13, 2023

Conversation

abravalheri
Copy link
Contributor

I think it makes sense to add a warning recommending the migration to PEP 420. pkg_resources is in its majority "discouraged", isn't it?

Summary of changes

  • Add a DeprecationWarning to pkg_resources.declare_namespace.
    • By default, Python will not show DeprecationWarning to the final user (unless they opt-in for something more strict, in which case they should be familiar with more complex warning filters), so this approach should be OK.

Closes

Pull Request Checklist

@pradyunsg
Copy link
Member

I reckon this is a good idea, but without a clear timeline or policy on what this deprecation means (beyond "use a newer nicer thing, instead of this older thing"), it's not particularly useful to "deprecate".

As a fly-by suggestion, consider doing the following:

  • providing documentation describing why this is being deprecated.
  • providing a rough timeline/metric/trigger point would be used to decide on the removal.
  • linking to these from the deprecation message being printed.

@abravalheri
Copy link
Contributor Author

Thank you very much @pradyunsg for the feedback.

I think that in practice pkg_resources.declare_namespace is already considered deprecated, together with the entire pkg_resources due to the intention announced in milestone 4. Adding a deprecation warning would be a matter of improving communication.

Unfortunately this milestone does not provide a timeline or metric/trigger point that we could reference here (I understand that it is a complicated matter).

Regarding your suggestions.

providing documentation describing why this is being deprecated.

That is a very fair request, thank you very much. I will try to come with something soon-ish...

providing a rough timeline/metric/trigger point would be used to decide on the removal.

I think I cannot come up with this alone. To be sincere it is not clear to me yet what is the setuptools policy about deprecation, and how much time we give as notice between deprecation and removal.

I also understand that the support for some features instead of being completely dropped can just be reduced to the bare minimum in order to avoid breaking old packages in the ecosystem (e.g. supported in regular installations, but not in editable installations). So this could be the way to go here.

I will wait for input from the other maintainers, and once we have an understanding/agreement on what would be a suitable timeline/metric/trigger, I can add that.

@pradyunsg
Copy link
Member

Fair enough! I guess my comments are more pertinent to the broader deprecation then, instead of this specific PR.

I appreciate that you took the time to write a detailed reply @abravalheri to my fly-by comment. ^.^

@jaraco
Copy link
Member

jaraco commented Feb 12, 2023

This PR seems reasonable. I was originally concerned that the deprecation warning might be noticed by end users, but then was reminded that DeprecationWarnings are typically not visible to end users except in test suites, so that's good. I'm fine with rolling this out to see how it helps.

One other thing to check - have we also deprecated the use of namespace_packages in setup()?

@abravalheri
Copy link
Contributor Author

I have rebased the PR and updated the docs to explain why the deprecation is needed.

One other thing to check - have we also deprecated the use of namespace_packages in setup()?

Yes, it is already deprecated: https://github.com/pypa/setuptools/blob/v67.2.0/setuptools/dist.py#L281-L285

@abravalheri abravalheri merged commit 925a792 into pypa:main Feb 13, 2023
@abravalheri abravalheri deleted the deprecate-declare-namespace branch February 13, 2023 20:57
@tacaswell
Copy link

Is there a good guide on how to actually do this migration on the downstream package side? Is it enough to just delete the __init__.py? Are there any expected gotchas and how should we prepare to communicate to any farther down stream packages?

@abravalheri
Copy link
Contributor Author

Removing the __init__.py does the trick.

The caveat is that all projects sharing the same namespace have to agree with that... (The same way, previously they all agreed to use pkg_resources).

@jaraco
Copy link
Member

jaraco commented Feb 18, 2023

Although technically, a namespace should share the same technique, in my experience in typical pip-installed environments (which strip the init.py for pkg_resources-managed namespace packages anyway), the transition is painless. I migrated several namespaces, including svg, jaraco, and now backports from pkg_resources-style to pkgutil-style (for Python 2 compatibility) to native (PEP 420) packages without any problems, even when the migration was staggered.

For example:

jaraco/jaraco.functools@d84738a
jaraco/jaraco.functools@1d0e764

If you encounter any trouble, don't hesitate to mention me in a bug report.

@gforcada
Copy link

gforcada commented Mar 7, 2023

That's going to be interesting for zope, plone and especially collective namespaces, as there are lots of packages to coordinate 😓

/cc @mauritsvanrees @icemac @dataflake

@icemac
Copy link
Contributor

icemac commented Mar 8, 2023

I am not sure if zc.buildout already supports PEP 420 namespaces but think we'll find out on the way.

@mauritsvanrees
Copy link
Sponsor Contributor

Hi, Plone Release Manager here.
Just as a data point: with a fresh Plone 6.0.2, which is one week old, I have these namespaces, with the amount of packages sharing it:

   3 five
   3 zc
   7 z3c
  27 Products
  49 zope
  82 plone

That is a total of 171 packages. Included in those are a few with a second namespace level:

   2 plone.formwidget
   2 plone.portlet
  34 plone.app

I don't look forward to fixing all of these in one go as community, but I guess it was coming, I think have seen a deprecation warning already a few months ago. If needs be, it can be done.

All packages that you use from one namespace have to use the same technique, right? So either pkg_resources namespaces or native namespace (ignoring pkgutil here for simplicity).

This means that for Plone it only makes sense to do this in a major release, so Plone 7. Since the previous major release was only released a few months ago, December 2022, it can still be a few years before the next major one is released. We have no definite timeline yet, but here is our release schedule.

Maybe this can be taken into account when considering when to completely remove support for pkg_resources namespaces? That would be nice.

The Plone policy can be summarised as this:

  • Each major release gets at least two years of maintenance support. For Plone 6 this is December 2024.
  • Each major release gets at least five years of security support. For Plone 6 this is December 2027.

Keeping support for pkg_resources namespaces for five more years seems too much to ask.
Can I ask for two more years? So drop it in the first setuptools release in 2025. Let me put that in a new comment, to make it easier to like or dislike that comment.

Of course, once support is dropped, people can still use an older setuptools version when they run Plone 6. I don't think there are any plans to ban either uploading or downloading pkg_resources-style namespace packages from PyPI.

@mauritsvanrees
Copy link
Sponsor Contributor

Can we keep support for pkg_resources namespace packages for two more years?
So drop it in the first setuptools release in 2025.

Note: This is not an official poll. I am not a setuptools maintainer. It is up to the maintainers to balance this request and maintainability and future plans.

@gforcada
Copy link

gforcada commented Mar 8, 2023

Where I see the major problem here is with collective namespace... We have [+1000k repositories sharing that namespace].

Coordinating Zope or Plone on its own will be a lot of effort, but doing that for collective namespace feels almost impossible 😕

As @mauritsvanrees says, knowing when the code is going to be removed to see how much time we have to adapt, and specially which version will be the last one with support for pkg_resource namespaces would be good to know, the sooner the better 🤞🏾 🍀

@abravalheri
Copy link
Contributor Author

Hi @mauritsvanrees and @gforcada thank you very much for the insights.

The idea behind introducing the warning was precisely to improve the communication with the community. This deprecation and alternative has been documented for a few years (together with other tips and advise for swapping pkg_resources to suitable alternatives), but I thought it was important to add a direct message to the dev, specially because pip 23.1 is going to be a major enabler for us1.

Setuptools still has to finish decoupling itself from pkg_resources, so the complete remove still is going to take some time.

We have a few options on how to handle the removal in a way that it is backwards compatible:

  • We could decouple the 2 code bases but still distribute pkg_resources together with setuptools for some time.
  • Once setuptools is independent from pkg_resources, we could create a separated pkg_resources package on PyPI for those that need to use the legacy API (as a frozen snapshot once we stop updating it).
    • I don't know if that would be a suitable solution for you. Do the packages have a common dependency that could require pkg_resources?
  • Maybe even other approaches ...

Probably @jaraco will have better insights on this topic.

It would be interesting also to understand to which extent the projects are relying on deprecated features of setuptools which are not exactly direct calls to pkg_resources.

Footnotes

  1. pip>=23.1 will use setuptools.build_meta:__legacy__ by default instead of calling python setup.py install (which needs pkg_resources).

@mauritsvanrees
Copy link
Sponsor Contributor

We have a few options on how to handle the removal in a way that it is backwards compatible:

  • We could decouple the 2 code bases but still distribute pkg_resources together with setuptools for some time.

So setuptools would vendor pkg_resources, but not call it itself? Then such a setuptools version would by definition not support pkg_resources-style namespaces, right? I suppose namespace_packages=['plone'] in setup.py would either give an error or it would generate native namespaces.

  • Once setuptools is independent from pkg_resources, we could create a separated pkg_resources package on PyPI for those that need to use the legacy API (as a frozen snapshot once we stop updating it).

For the same reason, I don't think this can technically work for pkg_resources namespaces.
Or would pkg_resources somehow hook into or override parts of setuptools to make this work?

  • I don't know if that would be a suitable solution for you. Do the packages have a common dependency that could require pkg_resources?

I suppose zope.interface is used by everything, so that could be a candidate.

It would be interesting also to understand to which extent the projects are relying on deprecated features of setuptools which are not exactly direct calls to pkg_resources.

Entry points are widely used in Plone, certainly for add-ons and some tooling packages like zest.releaser. That may need to be switched over to importlib.metadata.

In code outside of setup.py we often use pkg_resources.get_distribution to check if a distribution is available or not and do some extra things then. That is probably also food for importlib.metadata.

I recently patched code that had an incompatibility when calling pkg_resources.parse_version because of the more strict version parsing in setuptools 66+ or 67+. That may need to be switched to packaging.version.parse, but nothing depended on that yet, so the package was not available.

There are about seven Zope packages that use C extensions, but I don't think there is anything deprecated about that.

For the rest I am not aware of anything in Plone code that would be setuptools specific. I expect that there are a few setup.py files that have a bit of custom Python logic, which would not be called when there is no python setup.py call. But such dynamic behavior should not be used anyway.

@GDYendell
Copy link

GDYendell commented Mar 10, 2023

This will break previously working CI for packages that use pkg_resources.declare_namespace (or a dependency that does) and have filterwarnings = error set in pytest config, unless setuptools is pinned below 0.67.3.

A workaround is to add an ignore for this specific deprecation warning:

filterwarnings =
    error
    ignore:Deprecated call to `pkg_resources.declare_namespace:DeprecationWarning

https://docs.pytest.org/en/7.1.x/how-to/capture-warnings.html#controlling-warnings

Is this a reasonable thing to do until packages are updated?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants