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

conda-feedstock broken? #5128

Closed
drewgilliam opened this issue Jun 13, 2022 · 11 comments · Fixed by #5222
Closed

conda-feedstock broken? #5128

drewgilliam opened this issue Jun 13, 2022 · 11 comments · Fixed by #5222
Labels
Type: Question ❔ This is a question or a request for support.

Comments

@drewgilliam
Copy link

I've haven't gotten any response over at conda-forge, so I'm hoping someone here can help. The conda-forge feedstock has failed for almost all 2022 pipenv releases (latest pipenv version on conda-forge is 2022.1.8). Would anyone on the pipenv team be able to help fix the conda-forge feedstock?

conda-forge/pipenv-feedstock#24
https://anaconda.org/conda-forge/pipenv/files

@matteius
Copy link
Member

@drewgilliam Can you kick off some new builds over there? All of the old builds are too old to have the log files still available in Github Actions.

@drewgilliam
Copy link
Author

Thanks for the quick response! I don't think I have permissions to do anything in pipenv-feedstock - I'm just a user of conda-forge not an admin of the pipenv-feedstock. There seem to only be two human contributors in the history of pipenv-feedstock., @roccqqck and @beckermr, maybe they could kick something off?

@beckermr
Copy link

I don't recall making any non-admin changes to the feedstock. (I did accidentally make commits to most feedstocks on conda forge a while back.)

Feel free to push a fix.

@matteius
Copy link
Member

@beckermr Well for one, I don't know what the pipenv-feedstock project is about, and the actual problem has been lost to the logs being no longer available, so unless you can kick off a run that demonstrates the failure within the logs, then I won't be spending time on it.

@matteius matteius added the Type: Question ❔ This is a question or a request for support. label Jun 14, 2022
@drewgilliam
Copy link
Author

So I believe I figured out the problem, which boils down to an environment variable.
conda-forge/pipenv-feedstock#25

PR #4944 changed the way PIPENV_IS_CI is calculated

PIPENV_IS_CI = env_to_bool(os.environ.get('CI') or os.environ.get('TF_BUILD') or False)

In my understanding, this code expects/requires CI and TF_BUILD to be unset, empty, or boolean-ish (truthy/falsey strings). However, in pipenv_feedstock CI=azure and all pipenv commands fail. I can replicate the problem just by setting CI=anything on pretty much any machine..

$ CI=dummy pipenv --help
Traceback (most recent call last):
  File "/usr/local/bin/pipenv", line 5, in <module>
    from pipenv import cli
  File "/usr/local/pipenv/lib/python3.9/site-packages/pipenv/__init__.py", line 57, in <module>
    from .cli import cli
  File "/usr/local/pipenv/lib/python3.9/site-packages/pipenv/cli/__init__.py", line 1, in <module>
    from .command import cli  # noqa
  File "/usr/local/pipenv/lib/python3.9/site-packages/pipenv/cli/command.py", line 4, in <module>
    from pipenv import environments
  File "/usr/local/pipenv/lib/python3.9/site-packages/pipenv/environments.py", line 100, in <module>
    PIPENV_IS_CI = env_to_bool(os.environ.get("CI") or os.environ.get("TF_BUILD") or False)
  File "/usr/local/pipenv/lib/python3.9/site-packages/pipenv/environments.py", line 34, in env_to_bool
    raise ValueError(f"Value is not a valid boolean-like: {val}")
ValueError: Value is not a valid boolean-like: dummy

Is this expected behavior?

@matteius
Copy link
Member

Sorry for the delay in responding @drewgilliam, yes I believe this is expected that CI should be set to 1 or unset or 0. However at first glance, that code doesn't look correct. Shouldn't this line:
PIPENV_IS_CI = env_to_bool(os.environ.get("CI") or os.environ.get("TF_BUILD") or False)
Actually be more correct as this:
PIPENV_IS_CI = env_to_bool(os.environ.get("CI")) or env_to_bool(os.environ.get("TF_BUILD"))

cc: @oz123

@matteius
Copy link
Member

@drewgilliam Clarifying question -- does your environment need CI to be set to azure for other reasons in the build pipeline besides pipenv?

@drewgilliam
Copy link
Author

I don't know too much about the conda-forge feedstock environment - in my limited understanding, its a fairly automated environment where a lot of stuff is autogenerated for each feedstock package by the "conda smithy". The CI variable appears to be set in one of these auto-generated files, I assume/hope for good reason:
https://github.com/conda-forge/pipenv-feedstock/blob/main/.azure-pipelines/azure-pipelines-linux.yml#L32

Even with the suggested change, env_to_bool is going to throw an error if CI or TF_BUILD are anything but truthy/falsey.

_false_values = ("0", "false", "no", "off")
_true_values = ("1", "true", "yes", "on")
def env_to_bool(val):
"""
Convert **val** to boolean, returning True if truthy or False if falsey
:param Any val: The value to convert
:return: False if Falsey, True if truthy
:rtype: bool
"""
if isinstance(val, bool):
return val
if val.lower() in _false_values:
return False
if val.lower() in _true_values:
return True
raise ValueError(f"Value is not a valid boolean-like: {val}")

Assuming you're just trying to check if you're in a CI=1 type environment, maybe it would be better if env_to_bool returned false by default?

_true_values = ("1", "true", "yes", "on")

def env_to_bool(val):
    """
    Convert **val** to boolean, returning True only if truthy
    :param Any val: The value to convert
    :return: True if truthy, otherwise False
    :rtype: bool
    """
    if isinstance(val, bool):
        return val
    else:
        return val.lower() in _true_values

@matteius
Copy link
Member

@drewgilliam Is it safe to close this ticket at this point seeing as conda-feedstock has been fixed?

@drewgilliam
Copy link
Author

@matteius - before you close this issue, is there an answer to my comment above? It seems unexpected that pipenv will fail for any user/system that happens to have an environment variable CI which does not translate to a boolean variable.

@matteius
Copy link
Member

matteius commented Aug 5, 2022

@drewgilliam I opened a PR to improve handling of the CI variable: #5222

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Question ❔ This is a question or a request for support.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants