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

Metapackage not passing pip check #6

Closed
xylar opened this issue Feb 25, 2021 · 6 comments
Closed

Metapackage not passing pip check #6

xylar opened this issue Feb 25, 2021 · 6 comments

Comments

@xylar
Copy link

xylar commented Feb 25, 2021

I'll be happy to provide a conda env, etc. if necessary but this isn't exactly relevant to the issue.

I'm seeing failures in pip check for a package (apache-airflow-providers-microsoft-azure) I'm trying to build that has azure-keyvault as a dependency: conda-forge/staged-recipes#14087 I believe this happens because azure-keyvault is a conda metapackage, so pip check has no metadata to pick up and thinks the package is missing.

I'm not entirely sure what the solution is. For now, I was patching the dependency out of the package I'm trying to build, but that doesn't seem like a great fix. We are likely to run into this pretty often as we add pip check to more and more packages: conda-forge/conda-forge.github.io#962

@xylar
Copy link
Author

xylar commented Mar 26, 2022

More than a year later, this is still giving me trouble. #5 made changes that are not consistent with downstream packages, which expect the version installed from pip. No version 4.2.0 of that metapackage was ever released.

I think a solution for me would be to get myself added as a maintainer, then build v4.1.0 of the package using the pypi metapackage, then constrain my downstream packages not to use the v4.2.0 metapackage from #5.

@xylar xylar mentioned this issue Mar 26, 2022
4 tasks
@dhirschfeld
Copy link
Member

This package should not install the pip meta-package from pypi. conda and pip are different packaging ecosystems. A conda package shouldn't be installing pip packages behind the scenes.

This meta-pacakge will install underlying packages consistent with the pypi:

image

run:
- azure-keyvault-certificates ~=4.2.0
- azure-keyvault-keys ~=4.2.0
- azure-keyvault-secrets ~=4.2.0

There are nevertheless a couple of issues here. The version here doesn't align with upstream as it pins more strictly than upstream (to agree with the underlying package versions). This may well confuse pip check. There are many other instances where pip check doesn't work with conda packages and in these cases, IMO, the correct solution is to simply not use pip check. My position on pip check is that if it works, great but if not, so be it - it's a tool designed for a different ecosystem.

It appears that the plan with the legacy azure-keyvault meta-package is to never publish a new version as it will accept any underlying dependency in ~=4.0. This version specification is near useless for packaging and IMO can only be justified as a convenience for an end user. IMO actual packages should not depend on this under-specified meta-package.

In the real world though, I guess packages may be depending on this meta-package... so I'm not opposed to publishing a 4.1.0 aligned with the pypi pinnings (even if I think packages should depend on the new underlying packages rather than this meta-package).

@xylar
Copy link
Author

xylar commented Mar 27, 2022

@dhirschfeld, thanks for taking the time to share your perpsective on this.

First, I don't know if it's useful for us on conda-forge to "go rogue" and define a package (even a metapackage) differently from the upstream source (https://github.com/Azure/azure-sdk-for-python/tree/main/sdk/keyvault/azure-keyvault). That doesn't have anything to do with PyPI or pip check. It has to do with what packages that have azure-keyvault as a dependency are expecting.

Second, it isn't clear to me from the upstream package that the intention was to get consistent versions of the azure-keyvalut-* packages. There has not clearly been an effort to keep the version numbers in sync in general. azure-keyvault-keys has already released v4.4.0 and is well on the way to v4.5.0 (based on tags) whereas azure-keyvault-secrets and azure-keyvault-certificates are both still at v4.3.0.

Third, regarding pip check, I would agree that a conda-forge check or equivalent would be much preferable. pip check is a pretty underwhelming constraint on whether the dependencies have the right constraints. But lacking any other tool at the moment, it is very helpful particularly for relatively complicated packages like https://github.com/conda-forge/apache-airflow-providers-microsoft-azure-feedstock/blob/main/recipe/meta.yaml.

I am willing to continue to just patch out azure-keyvault from the dependencies so I can keep using pip check. I'm also fine with putting in the underlying dependencies instead of the metapackage in that process. I just don't think it's a sustainable strategy for conda-forge to diverge from the upstream packages (even metapackages).

@xylar xylar closed this as completed Mar 27, 2022
@dhirschfeld
Copy link
Member

4.1.0 has been published which should be consistent with the upstream 4.1.0 so if you specify azure-keyvault >=4.1 in your meta.yaml the environment it creates will be consistent with the specification in the setup.py. If pip check still isn't happy then it's probably just not compatible with conda meta-packages.

I had noticed the packages' development versions diverging so 4.3.0 just bumps the minimum version of the required packages and leaves the maximum constrained only by the major version. An environment created by installing the conda version 4.3.0 will also be consistent with 4.1.0 (as will one created by installing conda version 4.2.0) so if pip check complains I'm not sure there's anything we can do about it here.

If you'd like pip check to also work for conda I'd recommend putting in a PR upstream to not use the meta-package but to specify the individual packages instead:
https://github.com/apache/airflow/blob/0c30564992a9250e294106e809123f0d5b1c2b78/setup.py#L221

It appears they only actually use the azure-keyvault-secrets package so that could stop them installing 2 redundant packages:
https://github.com/apache/airflow/blob/388723950de9ca519108e0a8f6818f0fc0dd91d4/airflow/providers/microsoft/azure/secrets/key_vault.py#L24

@xylar
Copy link
Author

xylar commented Mar 27, 2022

Thanks for making v4.1.0 of this metapackage.

Indeed, that’s not going to help with pip check. It really does need the pypi version of the metapackage to be installed to recognize it.

But I realize we’re just not going to agree on how to solve that so I will continue to patch downstream to allow me to use pip check.

@xylar
Copy link
Author

xylar commented Mar 27, 2022

I submitted the suggestion you made: apache/airflow#2255

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

No branches or pull requests

2 participants