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

Ignore messages on STDERR unless there is a non-zero return code #176

Merged
merged 2 commits into from
Aug 17, 2021

Conversation

weaverba137
Copy link
Member

This PR fixes #174 by changing the logic of how messages printed on STDERR are handled by desiInstall. The new logic can be summarized by this snippet:

                    if len(err) > 0:
                        #
                        # Pass STDERR messages to the user, but do not
                        # raise an error unless the return code was non-zero.
                        #
                        if proc.returncode == 0:
                            message = ("Pip emitted messages on STDERR; these can probably be ignored:\n" +
                                       err)
                            self.log.warning(message)
                        else:
                            message = ("Potentially serious error detected during pip installation:\n" +
                                       err)
                            self.log.critical(message)
                            raise DesiInstallException(message)

@sbailey, I know you're out of the office. Maybe @tskisner can take a quick look? I think this will be pretty non-controversial though.

@weaverba137 weaverba137 self-assigned this Aug 2, 2021
@coveralls
Copy link

coveralls commented Aug 3, 2021

Coverage Status

Coverage decreased (-0.03%) to 66.629% when pulling 55c00fa on warn-on-stderr into ee25edc on master.

Copy link
Contributor

@sbailey sbailey left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good. I tested it with the case in #174 and it works as expected (printing a warning with the pip stderr message but not generating a CRITICAL-level error with a non-zero exit code).

@sbailey sbailey merged commit 129dc44 into master Aug 17, 2021
@sbailey sbailey deleted the warn-on-stderr branch August 17, 2021 21:53
@weaverba137
Copy link
Member Author

@sbailey, do we need to get this into a tag right away or is it non-urgent?

@sbailey
Copy link
Contributor

sbailey commented Aug 17, 2021

non-urgent, we can tag later at our leisure. Thanks.

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

Successfully merging this pull request may close these issues.

desiInstall pip warnings resulting in CRITICAL level errors
3 participants