Fix pip config parsing for multiple extra-index-urls#3125
Fix pip config parsing for multiple extra-index-urls#3125saikonen merged 8 commits intoNetflix:masterfrom
Conversation
When pip config contains multiple extra-index-urls, `pip config list` outputs them separated by literal `\n` characters. The previous regex only split on whitespace, so multiple URLs were treated as a single mangled value. Split on literal `\n` as well. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR fixes
Confidence Score: 5/5Safe to merge — the change is a narrow, well-tested regex fix with no impact on other code paths. The fix is minimal and clearly correct: it adds No files require special attention. Important Files Changed
Reviews (9): Last reviewed commit: "Merge branch 'master' into fix/pip-confi..." | Re-trigger Greptile |
Regression test for the literal \n splitting fix plus coverage for single index, extra-index, multiple-index priority, empty config, and config call failure scenarios. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cd6e827 to
5744f99
Compare
|
@savingoyal @saikonen please review and add your feedback. Simple one line change, shouldn't need a lot of time to review. |
|
Also make sure to setup the pre-commit hooks insce your code fails on the |
|
Also the appropriate location for this test file should be inside |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3125 +/- ##
=========================================
Coverage ? 27.95%
=========================================
Files ? 381
Lines ? 52347
Branches ? 9238
=========================================
Hits ? 14634
Misses ? 36778
Partials ? 935 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Fix Black formatting in pip.py by splitting long line (88 char limit) - Move test from test/unit/ to test/plugins/pip/ following repo structure - Add test/plugins/pip/__init__.py per plugin test organization pattern Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Replace unittest.mock.patch with monkeypatch fixture - Use MagicMock with monkeypatch.setattr() for patching - Add docstring to helper function - Add comment sections for clarity (setup, execute, assert) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
@talsperre @saikonen addressed all comments! |
|
@talsperre @saikonen Please review this |
Summary
Pip.indices()to correctly parse multipleextra-index-urlvalues from pip configpip config listoutputs them separated by literal\ncharacters (backslash + n, not actual newlines)\s+) only split on whitespace, so multiple URLs were treated as a single mangled value\nas an additional split delimiter:r"\\n|\s+"Tests
Non-Goals
AI Tool Usage
Used claude code to identify and fix this bug, I completely understand the changes made