Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 6, 2025

Consolidates Python package validation logic by extracting validation functions from pip.go into a dedicated pip_validation.go file, following the repository's domain-specific validation pattern.

Changes

  • Created pkg/workflow/pip_validation.go (178 lines)

    • Moved 4 validation functions: validatePythonPackagesWithPip(), validatePipPackages(), validateUvPackages(), validateUvPackagesWithPip()
    • Added dedicated logger pipValidationLog for validation-specific logging
  • Refactored pkg/workflow/pip.go (reduced from 258 to 111 lines)

    • Retained extraction functions only: extractPipPackages(), extractPipFromCommands(), extractUvPackages(), extractUvFromCommands()
    • Updated documentation to clarify extraction vs validation responsibilities
    • Removed unused imports (fmt, os, os/exec, console, logger)

Structure

pkg/workflow/
├── pip.go              # Python package extraction from commands/config
├── pip_validation.go   # Python package validation against PyPI
├── npm.go              # NPM package extraction and validation (similar pattern)
└── validation.go       # General validation (unchanged)

This mirrors the existing npm.go pattern and avoids adding to the already large validation.go (850+ lines).

Original prompt

This section details on the original issue you should resolve

<issue_title>[task] Move Python/pip validation functions from pip.go to validation.go</issue_title>
<issue_description>## Objective
Consolidate Python package validation logic by moving validation functions from pkg/workflow/pip.go to pkg/workflow/validation.go.

Context

Part of issue #3311 - pip.go contains multiple validation functions that should be centralized in validation.go. These are validation functions, not pip-specific operations.

Functions to Move

From pkg/workflow/pip.go:

  1. validatePipPackages() (lines 83-109)
  2. validateUvPackages() (lines 112-170)
  3. validatePythonPackagesWithPip() (if exists)
  4. validateUvPackagesWithPip() (if exists)

Approach

  1. Review each validation function in pip.go
  2. Move all validate* functions to validation.go
  3. Maintain clear separation between validation logic and pip operations
  4. Update all references throughout the codebase
  5. Verify pip.go retains only pip-specific operations
  6. Run comprehensive tests

Files to Modify

  • pkg/workflow/pip.go - Remove validation functions
  • pkg/workflow/validation.go - Add validation functions
  • Update any files calling these validation functions

Acceptance Criteria

  • All validation functions moved from pip.go to validation.go
  • pip.go contains only pip-specific operations (parsing, extraction)
  • validation.go properly organized with new functions
  • All references updated correctly
  • Unit tests pass (make test-unit)
  • Integration tests pass if applicable
  • Code properly formatted (make fmt)

Estimated Effort: 1-1.5 hours
Priority: High (Priority 1 - High Impact, Low Risk)
Related to: #3311
Related to #3311

AI generated by Plan Command for #3311</issue_description>

Comments on the Issue (you are @copilot in this section)

@pelikhan Move to own file but not to a mega validation file.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Move Python/pip validation functions to validation.go Refactor: Extract Python package validation to dedicated pip_validation.go Nov 6, 2025
Copilot AI requested a review from pelikhan November 6, 2025 05:43
@pelikhan pelikhan marked this pull request as ready for review November 6, 2025 10:29
Copilot AI review requested due to automatic review settings November 6, 2025 10:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors Python package validation code by separating validation logic from extraction logic. The validation functions (validatePipPackages, validateUvPackages, validatePythonPackagesWithPip, validateUvPackagesWithPip) are moved from pip.go to a new file pip_validation.go, while pip.go is simplified to focus solely on package extraction.

Key Changes

  • Created new pip_validation.go file containing all Python package validation logic
  • Updated pip.go to focus exclusively on package extraction functions
  • Updated documentation in both files to reflect their distinct responsibilities

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
pkg/workflow/pip_validation.go New file containing Python package validation functions for pip and uv packages using PyPI index
pkg/workflow/pip.go Simplified to contain only package extraction functions; removed all validation logic and updated documentation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +155 to +161
// Use uv pip show to check if package exists on PyPI
cmd := exec.Command("uv", "pip", "show", pkgName, "--no-cache")
_, err := cmd.CombinedOutput()

if err != nil {
// Package not installed, try to check if it's available
errors = append(errors, fmt.Sprintf("uv package '%s' validation requires network access or local cache", pkg))
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

The uv pip show command checks if a package is already installed locally, not if it exists on PyPI. This will fail for packages that aren't installed, even if they exist on PyPI. Consider using uv pip index or falling back to pip validation similar to the fallback pattern already implemented in lines 126-142.

Suggested change
// Use uv pip show to check if package exists on PyPI
cmd := exec.Command("uv", "pip", "show", pkgName, "--no-cache")
_, err := cmd.CombinedOutput()
if err != nil {
// Package not installed, try to check if it's available
errors = append(errors, fmt.Sprintf("uv package '%s' validation requires network access or local cache", pkg))
// Use uv pip index versions to check if package exists on PyPI
cmd := exec.Command("uv", "pip", "index", "versions", pkgName)
_, err := cmd.CombinedOutput()
if err != nil {
// Package not found on PyPI or network/cache issue
errors = append(errors, fmt.Sprintf("uv package '%s' not found on PyPI or validation requires network access", pkg))

Copilot uses AI. Check for mistakes.
@pelikhan pelikhan merged commit e4b19e2 into main Nov 6, 2025
65 of 69 checks passed
@pelikhan pelikhan deleted the copilot/move-validation-functions-to-validation-go branch November 6, 2025 10:35
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.

[task] Move Python/pip validation functions from pip.go to validation.go

2 participants