-
Notifications
You must be signed in to change notification settings - Fork 310
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
feat: Introduce compatibility with native namespace packages #1205
Conversation
I'll address the 'conventionalcommits' errors after there's some consent on the general approach. |
This seems reasonable to me but I think it's best for @parthea to sign off. |
Blocked on b/266148789 |
Is it blocked? This change is at least theoretically compatible with Python 2. It may have compatibility concerns with other |
This work is now unblocked and can be picked up. I'll sync up with the team on next steps. |
Since we can now drop support for 2.7, I understand the recommended solution is this one https://packaging.python.org/en/latest/guides/packaging-namespace-packages/#native-namespace-packages? |
python 2.7 is still supported in google-auth-library-python/setup.py Line 70 in 1e6f7d0
|
Yes, that's what I would recommend. But here's the caveat: If there are other packages installed by the user that implement Does anyone know if there are other public packages that install behavior into the Also, would you like me to amend this PR to utilize the native namespace packages instead? |
I'll start work on removing Python 2.7 support in setup.py. Is it right that the level of risk between the native namespace and the solution in the PR is the same? I think this will have to be marked as a breaking change (correct me if I am wrong @parthea)? My preference is to go ahead with the native namespace, assuming the risk is the same. |
With Python 2.7 support removed, I believe the answer is yes. There are potentially (likely) rare, subtle edge cases, but for the most part, the risks are comparable. Since the
My instinct is to mark it as a breaking change and to indicate that the breaking aspect is ignorable for most users but that the package should not be installed alongside other packages supplying I'll update the PR to this new expectation. |
The latest commit proposes instead to use native namespace packages for "google", eliminating the I'm not confident that's the correct invocation, because setuptools will pick up any directory as a package (possibly only if it has .py files in it). That means that paths like We'll want to experiment/test to ensure consistency and correctness with this new change. |
The issue can be illustrated thus:
It's the |
If I understand correctly, I'm okay with pre-emptively excluding the How can I assist in testing this feature? The PR looks good to me, and the approach seems sound. |
The change includes a test, so the basic functionality is protected by CI. Where the biggest uncertainly lies in how this change interacts with other libraries in the I did some testing to check and indeed the packages seem to work well together in the same environment when installed by pip:
Even though google.cloud still has the pkg_resources namespace packaging behavior:
But something else I'm noticing now, even the pkg_resources-managed namespace packages, when installed with pip, are showing up as PEP 420 namespace packages (note
That indicates to me that the compatibility story has improved from years ago when the analysis was done on cross-compatibility. I do note, however, that other installation methods (older versions of pip or other installers) may produce different results. Feel free to use these explorations as inspiration for other ways to test the downstream effects of this change. |
This short chain demonstrates a missed expectation, that the
google-auth
package will mask the presence of other packages in agoogle
namespace when installed. This issue affects corporate packages internally.The
pkg_resources
style namespace is the least preferable and least compatible technique indicated in the docs.By removing the
pkg_resources
technique, it makes thepkgutil
technique the default, adds compatibility with native namespace packages.After Python 2.7 support is dropped, this package could consider dropping the
pkgutil
technique as well and instead use native namespace packages, but that's a consideration for a later time.