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

Filter out pip freeze output starting w/ "[notice]" #508

Merged
merged 5 commits into from
Oct 23, 2023

Conversation

sagerb
Copy link
Contributor

@sagerb sagerb commented Oct 18, 2023

Intent

The std output executed from the command of pip freeze is used to generate a project's requirements.txt file. As part of pip's standard functionality, checks for new version availability are executed "intermittently". When new versions are detected, the std output line starts with [notice]. For example:

[notice] A new release of pip is available: 23.1.2 -> 23.3
[notice] To update, run: pip install --upgrade pip

Since every line from the std output that does not contain reconnect is written into requirements.txt, the scenario where an upgrade is detected will cause the generated requirements.txt file to be invalid.

Resolves #270

Type of Change

  • Bug Fix
  • New Feature
  • Breaking Change

Approach

Added filter to suppress the std output lines, which start with the text [notice].

Refactored filtering to allow testability.

Added test for filtering std output which contains and doesn't contain a line starting with `[notice]'

Automated Tests

The new unit tests should cover validation.

Directions for Reviewers

It is challenging to force the pip upgrade notices to occur. The best approach for validation is probably continuous no-harm validation.

Checklist

  • I have updated CHANGELOG.md to cover notable changes.
  • I have updated all related GitHub issues to reflect their current state.

@sagerb sagerb requested a review from mmarchetti October 18, 2023 23:55
@sagerb sagerb self-assigned this Oct 18, 2023
Copy link
Contributor

@mmarchetti mmarchetti left a comment

Choose a reason for hiding this comment

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

The change looks good. We should also pass the --disable-pip-version-check version to pip whenever we run it non-interactively.

@sagerb
Copy link
Contributor Author

sagerb commented Oct 19, 2023

The change looks good. We should also pass the --disable-pip-version-check version to pip whenever we run it non-interactively.

I wondered about that earlier when I was formulating my strategy. I'll go ahead and add it.

--disable-pip-version-check version
@github-actions
Copy link

github-actions bot commented Oct 19, 2023

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
4240 2784 66% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
rsconnect/environment.py 82% 🟢
rsconnect/main.py 56% 🟢
TOTAL 69% 🟢

updated for commit: 4cfbdf5 by action🐍

@sagerb sagerb requested a review from mmarchetti October 19, 2023 16:51
@kgartland-rstudio
Copy link
Collaborator

I have since got a new computer and can no longer reproduce the original issue even with the same version of Python and Pip. Instead I tried to excerise pip in every way I could think of with an older version of pip (22.1.2) and the current version of pip (23.3).

Tested with various content types.

  • write-manifest --force-generate
  • deploy --force-generate
  • deploy (without requirements.txt)
  • deploy (with requirements.txt)

@sagerb sagerb merged commit f11851c into master Oct 23, 2023
14 checks passed
@sagerb sagerb deleted the sagerb-handle-pip-notices branch October 23, 2023 18:29
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.

Error on pip warning message
3 participants