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

azure-synapse-nspkg package is not an empty package for Python3 (PEP420) #13441

Closed
glaubitz opened this issue Aug 31, 2020 · 9 comments
Closed
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Synapse

Comments

@glaubitz
Copy link

I just packaged azure-synapse-nspkg for openSUSE/SLE and noticed the package is using the old namespace package format instead of the new one which creates an empty package on Python3 according to PEP420.

Compare: https://github.com/Azure/azure-sdk-for-python/blob/master/sdk/synapse/azure-synapse-nspkg/setup.py

and: https://github.com/Azure/azure-sdk-for-python/blob/master/sdk/core/azure-mgmt-nspkg/setup.py

The setup.py of azure-synapse-accesscontrol, however, refers to the namespace package using PEP420:

    packages=find_packages(exclude=[
        'tests',
        # Exclude packages that will be covered by PEP420 or nspkg
        'azure',
        'azure.synapse',
    ]),

From: https://github.com/Azure/azure-sdk-for-python/blob/master/sdk/synapse/azure-synapse-accesscontrol/setup.py#L76

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Aug 31, 2020
@kaerm kaerm added Service Attention Workflow: This issue is responsible by Azure service team. Synapse labels Aug 31, 2020
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Aug 31, 2020
@ghost
Copy link

ghost commented Aug 31, 2020

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @aim-for-better, @idear1203.

@idear1203
Copy link
Contributor

@lmazuel , I am not quite familiar with Python. Could you please help me understand what I should do here?

@lmazuel
Copy link
Member

lmazuel commented Sep 11, 2020

Hi @glaubitz
The nspkg is an optional dependency for Python 3, which mean azure-synapse-accesscontrol will in practical term use PEP420, since azure-synapse-nspkg won't be installed. This makes the actual content of azure-synapse-nspkg not really important on Python3 (it's why I simplified the packaging compared to azure-mgmt-nspkg). Does that answer your question?

@lmazuel lmazuel added needs-author-feedback Workflow: More information is needed from author to address the issue. and removed Service Attention Workflow: This issue is responsible by Azure service team. labels Sep 11, 2020
@lmazuel lmazuel self-assigned this Sep 11, 2020
@ghost ghost added the needs-team-triage Workflow: This issue needs the team to triage. label Sep 11, 2020
@lmazuel lmazuel added Client This issue points to a problem in the data-plane of the library. and removed needs-team-triage Workflow: This issue needs the team to triage. labels Sep 11, 2020
@glaubitz
Copy link
Author

Hi @glaubitz
The nspkg is an optional dependency for Python 3, which mean azure-synapse-accesscontrol will in practical term use PEP420, since azure-synapse-nspkg won't be installed. This makes the actual content of azure-synapse-nspkg not really important on Python3 (it's why I simplified the packaging compared to azure-mgmt-nspkg). Does that answer your question?

Well, I'm not happy with that solution because:

  1. It's inconsistent with the rest of the -nspkg packages
  2. We're always packaging all Azure packages in openSUSE/SLE, including the -nspkg as our package builds still support Python2 and Python3, so the package gets actually built for Python3 and is now different from the other -nspkg packages which is why I had to modify the partially automated spec file
  3. There isn't much work being saved when using the same mechanism as the other -nspkg package files, is there?
  4. Last time I reported an -nspkg that was not converted to implicit namespaces, the issue got actually fixed (azure-cognitiveservices-nspkg has not been converted to implicit namespaces (PEP 420) #4461)

@ghost ghost added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-author-feedback Workflow: More information is needed from author to address the issue. labels Sep 12, 2020
@lmazuel
Copy link
Member

lmazuel commented Sep 14, 2020

Hi @glaubitz
There is actually some work saved, since this way makes the wheel universal (and we don't have to release both a py2 and a py3 wheel). Yes, it's inconsistent with the current nspkg, and yes I changed my mind compared to #4461: it's a year later with some experience and feedback from other parties in the meantime.

What I don't understand, is why this is a problem on your build side that all nspkg are not precisely the same? I can never guarantee that all all nspkg will ever be strictly identical, as long as the behavior is equivalent here (especially since this package is not supposed to be installed in the first in Python 3). Thoughts?

@lmazuel lmazuel added needs-author-feedback Workflow: More information is needed from author to address the issue. and removed needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team labels Sep 16, 2020
@ghost ghost added the no-recent-activity There has been no recent activity on this issue. label Sep 24, 2020
@ghost
Copy link

ghost commented Sep 24, 2020

Hi, we're sending this friendly reminder because we haven't heard back from you in a while. We need more information about this issue to help address it. Please be sure to give us your input within the next 7 days. If we don't hear back from you within 14 days of this comment the issue will be automatically closed. Thank you!

@glaubitz
Copy link
Author

What I don't understand, is why this is a problem on your build side that all nspkg are not precisely the same? I can never guarantee that all all nspkg will ever be strictly identical, as long as the behavior is equivalent here (especially since this package is not supposed to be installed in the first in Python 3). Thoughts?

It's a problem because packaging is partially performed in an automated manner due to the large number of packages in the Azure SDK.

So, whenever possible packages should follow a common scheme. Anything that deviates needs manual intervention which costs time and slows down the process of keeping the packages in openSUSE/SLE up-to-date.

That's why it would be great if the nspkg packages were consistent.

@ghost ghost added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-author-feedback Workflow: More information is needed from author to address the issue. no-recent-activity There has been no recent activity on this issue. labels Sep 24, 2020
@lmazuel
Copy link
Member

lmazuel commented Oct 16, 2020

@glaubitz my current recommendation is to keep the synapse way, because:

  • It's actually more work to release a non-universal wheel (files in Py2, but none in Py3) and our release pipeline is not set-up for that
  • The fact that you don't need the nspkg on Python3, doesn't mean you could not want to install it for some reasons. If I make it empty I give no choice to people. So I'm untangling the "no-need" for this package to the actual content of the package
  • Recommendation evolves based on setuptools, pip, and issues reported by people. I tried for instance at once occasion to make this package "Python 2" only with "python requires", but the way pip was implementing this was breaking some cache system like Nexus. It's a difficult spot to find something that makes everybody happy unfortunately :/

I'm closing this issue since I won't change this nspkg, I'm happy to discuss by email this matter if necessary (you know how to contact me ;)). I want to be an ally as much as I can.

Thanks,

@lmazuel lmazuel closed this as completed Oct 16, 2020
@glaubitz
Copy link
Author

@lmazuel > my current recommendation is to keep the synapse way

I'm fine with that. It would be great then if the existing namespace packages which use implicit namespaces are reverted back to this scheme.

Again, I just care for the packages being consistent which makes my job of packaging the rising number of packages easier just as all packages switching over to CHANGELOG.md and README.md as opposed to some having a HISTORY.rst and some having no changelog file etc.

If you make the packages as consistent as possible, I'm fine with any design you pick :).

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Synapse
Projects
None yet
Development

No branches or pull requests

4 participants