Python metric collection

[sachin-sandhu]

What are you trying to accomplish?

Fix: Enables metric collection for pip-compile and pipenv
Enhancements to package manager detection:

Also enables local python setup for correct detection of pip-compile, poetry and pipenv based of python version

Anything you want to highlight for special attention from reviewers?

How will you know you've accomplished your goal?

Checklist

• I have run the complete test suite to ensure all tests and linters pass.
• I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
• I have written clear and descriptive commit messages.
• I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
• I have ensured that the code is well-documented and easy to understand.

[abdulapopoola]

why return untyped?

[sachin-sandhu]

@abdulapopoola , This is for fail safe , As we are extracting package manager version from shell command, in case the output changes or there is any issue in version extraction (very low possibility) we can make sure metric collection exception does not cause update to fail. We are checking for untyped at "return PoetryPackageManager.new(detect_poetry_version) if poetry_files && detect_poetry_version"

[pavera]

I am pretty sure there are python package managers we don't support yet, I think this should still follow the same logic and only return pip if pip is actually found. This method should return nil if no supported package managers are found.

[kbukum1]

I am pretty sure there are python package managers we don't support yet, I think this should still follow the same logic and only return pip if pip is actually found. This method should return nil if no supported package managers are found.

@pavera
When you return ecosystem object in file_parser, currently package_manager is required field. If we are unable detect package_manager I believe we shouldn't be able to hit this method since this method is getting called before starting update operation after parsing dependency files. In this situation we should return error or something for not supported package manager.

@sachin-sandhu
Is there a way to be sure that we are returning the right package manager?

[pavera]

the *_files checks are redundant here as the detect_*_version methods already check for the existence of the files.

[abdulapopoola]

should these be constants? Tagging @kbukum1 since he established a pattern in npm

[kbukum1]

Hi @sachin-sandhu,

I recommend checking the implementation of the npm_and_yarn package manager. You'll notice that we've moved all string constants into their respective modules/classes, allowing them to be reused throughout the system. This approach ensures that the code is cleaner and easier to maintain.

For example, in the following snippet, constants are defined within their relevant ecosystem or package manager classes. This keeps the code modular and avoids hardcoding strings repeatedly across the system:

module Dependabot
  module NpmAndYarn
    ECOSYSTEM = "npm_and_yarn"
    MANIFEST_FILENAME = "package.json"
    ...
    MANIFEST_PACKAGE_MANAGER_KEY = "packageManager"
    MANIFEST_ENGINES_KEY = "engines"

    class NpmPackageManager < Ecosystem::VersionManager
      extend T::Sig
      NAME = "npm"
      RC_FILENAME = ".npmrc"
      LOCKFILE_NAME = "package-lock.json"
      SHRINKWRAP_LOCKFILE_NAME = "npm-shrinkwrap.json"
      ...
    end

    class YarnPackageManager < Ecosystem::VersionManager
      extend T::Sig
      NAME = "yarn"
      RC_FILENAME = ".yarnrc"
      RC_YML_FILENAME = ".yarnrc.yml"
      LOCKFILE_NAME = "yarn.lock"
      ...
    end

    class PNPMPackageManager < Ecosystem::VersionManager
      extend T::Sig
      NAME = "pnpm"
      LOCKFILE_NAME = "pnpm-lock.yaml"
      PNPM_WS_YML_FILENAME = "pnpm-workspace.yaml"
      ...
    end

    DEFAULT_PACKAGE_MANAGER = NpmPackageManager::NAME

    # Define a type alias for the expected class interface
    NpmAndYarnPackageManagerClassType = T.type_alias do
      T.any(
        T.class_of(Dependabot::NpmAndYarn::NpmPackageManager),
        T.class_of(Dependabot::NpmAndYarn::YarnPackageManager),
        T.class_of(Dependabot::NpmAndYarn::PNPMPackageManager)
      )
    end
  end
end

By organizing constants this way, you can easily reference them across the system while maintaining a clean and structured design. Let me know if you have any questions!

Reference: https://github.com/dependabot/dependabot-core/blob/main/npm_and_yarn/lib/dependabot/npm_and_yarn/package_manager.rb#L11