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

mark noarch/nteract-scrapbook-0.5.0-*_1 broken #239

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Mar 8, 2021

Guidelines for marking packages as broken:

  • We prefer to patch the repo data (see here)
    instead of marking packages as broken. This alternative workflow makes environments more reproducible.
  • Packages with requirements/metadata that are too strict but otherwise work are
    not technically broken and should not be marked as such.
  • Packages with missing metadata can be marked as broken on a temporary basis
    but should be patched in the repo data and be marked unbroken later.
  • In some cases where the number of users of a package is small or it is used by
    the maintainers only, we can allow packages to be marked broken more liberally.
  • We (conda-forge/core) try to make a decision on these requests within 24 hours.

Checklist:

  • Make sure your package is in the right spot (broken/* for adding the
    broken label, not_broken/* for removing the broken label)
  • Added a description of the problem with the package in the PR description.
  • Added links to any relevant issues/PRs in the PR description.
  • Pinged the team for the package for their input.

The upstream package changed the distribution name (dropping the nteract- prefix). I added an alias package for depending on the old name, but it's causing issues for downstreams.

@conda-forge/nteract-scrapbook

@bollwyvl bollwyvl requested a review from a team as a code owner March 8, 2021 13:39
@beckermr
Copy link
Member

beckermr commented Mar 8, 2021

Removing a package to avoid a pip check failure is not really needed.

If pip check fails, that's fine. Conda is not pip and the metadata could simply be different.

Copy link
Member

@beckermr beckermr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pr is not needed.

@beckermr
Copy link
Member

beckermr commented Mar 8, 2021

The solution here is to remove pip check from the recipe of the offending feedstock. Or you can write some code to ignore certain errors.

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Mar 8, 2021

The problem is I (as @conda-forge/nteract-scrapbook) created a package which does not exist on pip (nteract-scrapbook 0.5.0) in the thinking that it would be easier to keep working in the future, but this is not the case. In this case, we're dealing with airflow-related stuff, which is all but impossible to test in any thorough way, and keep on top of the huge list of dependencies, which pip check at least lets us do.

I concur that having a conda-forge-specific pip check (which didn't require pip!) would be more useful in the long run... except that this metadata actually does get used at runtime by certain parts of the python (not pip) system, namely entry_points, which actually check that all of this stuff is consistent. We've encountered this on python-language-server, pytest, etc. which make significant use of that packaging feature.

@beckermr
Copy link
Member

beckermr commented Mar 8, 2021

I don't personally find this argument convincing. I'm happy to keep chatting though.

cc @conda-forge/core for other opinions

We should be able to create a simple feedstock that holds a python script that runs pip check and then ignores some errors selectively. This tool could then be used by a lot of recipes. This is a much better solution than pulling a package simply because it got renamed on conda.

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

Successfully merging this pull request may close these issues.

2 participants