From b322968c7248519b6d604ccfd53315c235ebac73 Mon Sep 17 00:00:00 2001 From: Jeff Widman Date: Sat, 22 Jul 2023 16:05:48 -0700 Subject: [PATCH 1/4] Instrument Python version metric We want to know what versions of python are required by our users, so this is an initial stab at collecting some data. Ideally we capture the minimum version, the maximum version, and the raw data from each manifest file that allowed us to calculate the min/max... unfortunately this isn't so clean/polished, but we need to start somewhere and even a primitive form of this metric will provide enough information for us to understand the user impact of dropping python 3.6. --- python/lib/dependabot/python/file_fetcher.rb | 21 ++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/python/lib/dependabot/python/file_fetcher.rb b/python/lib/dependabot/python/file_fetcher.rb index ac2c1877010..78b8d440109 100644 --- a/python/lib/dependabot/python/file_fetcher.rb +++ b/python/lib/dependabot/python/file_fetcher.rb @@ -37,6 +37,27 @@ 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. + 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. + "raw" => language_version_manager.user_specified_python_version || "unknown" + "max" => language_version_manager.python_major_minor || "unknown" + } + } + } + end + private def fetch_files From 89c5f202bea31dd247eb5243f10d535ce3253f04 Mon Sep 17 00:00:00 2001 From: Tom Christensen Date: Tue, 25 Jul 2023 07:26:37 -0600 Subject: [PATCH 2/4] add a missing comma --- python/lib/dependabot/python/file_fetcher.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/lib/dependabot/python/file_fetcher.rb b/python/lib/dependabot/python/file_fetcher.rb index 78b8d440109..fade2a658a2 100644 --- a/python/lib/dependabot/python/file_fetcher.rb +++ b/python/lib/dependabot/python/file_fetcher.rb @@ -51,7 +51,7 @@ def ecosystem_versions # 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. - "raw" => language_version_manager.user_specified_python_version || "unknown" + "raw" => language_version_manager.user_specified_python_version || "unknown", "max" => language_version_manager.python_major_minor || "unknown" } } From 96306ed582da4a894e5d6835d69f4153ce540c52 Mon Sep 17 00:00:00 2001 From: Tom Christensen Date: Tue, 25 Jul 2023 07:53:42 -0600 Subject: [PATCH 3/4] lint cleanup --- python/lib/dependabot/python/file_fetcher.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/python/lib/dependabot/python/file_fetcher.rb b/python/lib/dependabot/python/file_fetcher.rb index fade2a658a2..13e96986952 100644 --- a/python/lib/dependabot/python/file_fetcher.rb +++ b/python/lib/dependabot/python/file_fetcher.rb @@ -47,10 +47,11 @@ def ecosystem_versions 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. + 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. "raw" => language_version_manager.user_specified_python_version || "unknown", "max" => language_version_manager.python_major_minor || "unknown" } From 5f524ed9de722b87a5d996271487070dbb327a41 Mon Sep 17 00:00:00 2001 From: Jeff Widman Date: Tue, 25 Jul 2023 13:52:18 -0700 Subject: [PATCH 4/4] add basic unit test --- python/spec/dependabot/python/file_fetcher_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/python/spec/dependabot/python/file_fetcher_spec.rb b/python/spec/dependabot/python/file_fetcher_spec.rb index 1570aabd413..2ee7e6ddd2e 100644 --- a/python/spec/dependabot/python/file_fetcher_spec.rb +++ b/python/spec/dependabot/python/file_fetcher_spec.rb @@ -507,6 +507,12 @@ expect(file_fetcher_instance.files.count).to eq(2) expect(file_fetcher_instance.files.map(&:name)).to include("setup.cfg") end + + it "exposes the expected ecosystem_versions metric" do + expect(file_fetcher_instance.ecosystem_versions).to eq({ + languages: { python: { "max" => "3.11", "raw" => "unknown" } } + }) + end end context "with a requirements.txt, a setup.py and a requirements folder" do