Skip to content

Commit

Permalink
Switch to the record_ecosystem_versions endpoint
Browse files Browse the repository at this point in the history
This flips the `updater` to calling the new `record_ecosystem_versions`
endpoint.

Note that this does _not_ flip the per-ecosystem instances of those
metrics. That will be done in separate PR's for easier
review/discussion/audit of those individual ecosystems.

Co-authored-by: Barry Gordon <[email protected]>
  • Loading branch information
jeffwidman and brrygrdn committed Jul 4, 2023
1 parent cd72277 commit ee51f3c
Show file tree
Hide file tree
Showing 9 changed files with 19 additions and 25 deletions.
4 changes: 2 additions & 2 deletions bin/dry-run.rb
Original file line number Diff line number Diff line change
Expand Up @@ -799,8 +799,8 @@ def security_fix?(dependency)
StackProf.results("tmp/stackprof-#{Time.now.strftime('%Y-%m-%d-%H:%M')}.dump") if $options[:profile]

puts "🌍 Total requests made: '#{$network_trace_count}'"
package_manager = fetcher.package_manager_version
puts "🎈 Package manager version log: #{package_manager}" unless package_manager.nil?
ecosystem_versions = fetcher.ecosystem_versions
puts "🎈 Ecosystem Versions log: #{ecosystem_versions}" unless ecosystem_versions.nil?

# rubocop:enable Metrics/BlockLength

Expand Down
2 changes: 1 addition & 1 deletion common/lib/dependabot/file_fetchers/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def clone_repo_contents
raise Dependabot::RepoNotFound, source
end

def package_manager_version
def ecosystem_versions
nil
end

Expand Down
9 changes: 3 additions & 6 deletions updater/lib/dependabot/api_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,10 @@ def update_dependency_list(dependencies, dependency_files)
sleep(rand(3.0..10.0)) && retry
end

def record_package_manager_version(package_managers)
api_url = "#{base_url}/update_jobs/#{job_id}/record_package_manager_version"
def record_ecosystem_versions(ecosystem_versions)
api_url = "#{base_url}/update_jobs/#{job_id}/record_ecosystem_versions"
body = {
data: {
ecosystem: "deprecated",
"package-managers": package_managers
}
data: { ecosystem_versions: ecosystem_versions }
}
response = http_client.post(api_url, json: body)
raise ApiError, response.body if response.code >= 400
Expand Down
7 changes: 2 additions & 5 deletions updater/lib/dependabot/file_fetcher_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,9 @@ def perform_job
raise "base commit SHA not found" unless @base_commit_sha

# We don't set this flag in GHES because there's no point in recording versions since we can't access that data.
# TODO: The flag is named `record_ecosystem_versions` because `package_manager_version` is getting renamed to
# to that shortly... but splitting into separate PR's for lower risk/easiery testability. Once the follow-on PR
# with the rename lands, the name confusion will disappear.
if Experiments.enabled?(:record_ecosystem_versions)
version = file_fetcher.package_manager_version
api_client.record_package_manager_version(version[:package_managers]) unless version.nil?
ecosystem_versions = file_fetcher.ecosystem_versions
api_client.record_ecosystem_versions(ecosystem_versions) unless ecosystem_versions.nil?
end

dependency_files
Expand Down
2 changes: 1 addition & 1 deletion updater/lib/dependabot/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def initialize(client:)
def_delegators :client,
:mark_job_as_processed,
:update_dependency_list,
:record_package_manager_version,
:record_ecosystem_versions,
:increment_metric

def create_pull_request(dependency_change, base_commit_sha)
Expand Down
6 changes: 3 additions & 3 deletions updater/spec/dependabot/api_client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -384,12 +384,12 @@
end
end

describe "record_package_manager_version" do
let(:url) { "http://example.com/update_jobs/1/record_package_manager_version" }
describe "ecosystem_versions" do
let(:url) { "http://example.com/update_jobs/1/record_ecosystem_versions" }
before { stub_request(:post, url).to_return(status: 204) }

it "hits the correct endpoint" do
client.record_package_manager_version({ "bundler" => "2" })
client.record_ecosystem_versions({ "ruby" => { "min" => 3, "max" => 3.2 } })

expect(WebMock).
to have_requested(:post, url).
Expand Down
5 changes: 3 additions & 2 deletions updater/spec/dependabot/file_fetcher_command_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

allow(api_client).to receive(:mark_job_as_processed)
allow(api_client).to receive(:record_update_job_error)
allow(api_client).to receive(:record_package_manager_version)
allow(api_client).to receive(:record_ecosystem_versions)

allow(Dependabot::Environment).to receive(:output_path).and_return(File.join(Dir.mktmpdir, "output.json"))
allow(Dependabot::Environment).to receive(:job_id).and_return(job_id)
Expand Down Expand Up @@ -109,8 +109,9 @@
allow_any_instance_of(Dependabot::Bundler::FileFetcher).
to receive(:files).
and_raise(exception)
# TODO: this may be deletable since the ecosystem_versions call is now buried behind a flag...
allow_any_instance_of(Dependabot::Bundler::FileFetcher).
to receive(:package_manager_version).
to receive(:ecosystem_versions).
and_return(nil)
end

Expand Down
7 changes: 3 additions & 4 deletions updater/spec/dependabot/integration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,15 @@
mark_job_as_processed: nil,
update_dependency_list: nil,
record_update_job_error: nil,
record_package_manager_version: nil,
record_ecosystem_versions: nil,
increment_metric: nil)
end
let(:file_fetcher) do
instance_double(Dependabot::FileFetchers::Base,
files: dependency_files,
commit: "sha",
package_manager_version: nil)
# TODO: this may be deletable since the ecosystem_versions call is now buried behind a flag...
ecosystem_versions: nil)
end
let(:message_builder) do
instance_double(Dependabot::PullRequestCreator::MessageBuilder, message: nil)
Expand All @@ -79,8 +80,6 @@

# Stub Dependabot object with instance doubles
allow(Dependabot::ApiClient).to receive(:new).and_return(api_client)
# Recording the package manager happens via an observer so the instantiated `api_client` does not receive this call
allow_any_instance_of(Dependabot::ApiClient).to receive(:record_package_manager_version)
allow(Dependabot::FileFetchers).to receive_message_chain(:for_package_manager, :new).and_return(file_fetcher)
allow(Dependabot::PullRequestCreator::MessageBuilder).to receive(:new).and_return(message_builder)

Expand Down
2 changes: 1 addition & 1 deletion updater/spec/dependabot/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
describe "Instance methods delegated to @client" do
{
mark_job_as_processed: %w(mock_sha),
record_package_manager_version: %w(mock_package_managers)
record_ecosystem_versions: %w(mock_ecosystem_versions)
}.each do |method, arguments|
before { allow(mock_client).to receive(method) }

Expand Down

0 comments on commit ee51f3c

Please sign in to comment.