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

Instrument Python version metric #7617

Merged
merged 4 commits into from
Jul 25, 2023
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 22 additions & 0 deletions python/lib/dependabot/python/file_fetcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,28 @@ def self.required_files_message
"or a Pipfile."
end

def ecosystem_versions
# Hmm... it's weird that this calls file parser methods, but here we are in the file fetcher... for all
# ecosystems our goal is to extract the user specified versions, so we'll need to do file parsing... so should
# we move this `ecosystem_versions` metrics method to run in the file parser for all ecosystems? Downside is if
# file parsing blows up, this metric isn't emitted, but reality is we have to parse anyway... as we want to know
# the user-specified range of versions, not the version Dependabot chose to run.
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm planning to have follow-on PR's that move this to the FileParser class, but that refactor will require more work, and want this live now so it starts collecting metrics, so I'm going to merge as-is and then clean it up.

python_requirement_parser = FileParser::PythonRequirementParser.new(dependency_files: files)
language_version_manager = LanguageVersionManager.new(python_requirement_parser: python_requirement_parser)
{
languages: {
python: {
# TODO: alternatively this could use `python_requirement_parser.user_specified_requirements` which
# returns an array... which we could flip to return a hash of manifest name => version
# string and then check for min/max versions... today it simply defaults to
# array.first which seems rather arbitrary.
Copy link
Member Author

Choose a reason for hiding this comment

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

user_specified_requirements is a private method, so the best way to do that is move this to FileParser, but again, we want metrics soon, so ship this first then clean that up.

"raw" => language_version_manager.user_specified_python_version || "unknown",
"max" => language_version_manager.python_major_minor || "unknown"
}
}
}
end

private

def fetch_files
Expand Down