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.
  • Loading branch information
jeffwidman committed Jul 4, 2023
1 parent 00b6bdf commit c2c28d2
Show file tree
Hide file tree
Showing 9 changed files with 15 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?
e_versions = fetcher.ecosystem_versions
puts "🎈 Ecosystem Versions log: #{e_versions}" unless e_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(ecosystem, 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: ecosystem,
"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[:ecosystem], version[:package_managers]) unless version.nil?
e_versions = file_fetcher.ecosystem_versions
api_client.record_ecosystem_versions(e_versions) unless e_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
8 changes: 3 additions & 5 deletions updater/spec/dependabot/api_client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -384,14 +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", { "bundler" => "2" }
)
client.record_ecosystem_versions({ "ruby" => { "min" => 3, "max" => 3.2 }})

expect(WebMock).
to have_requested(:post, url).
Expand Down
2 changes: 1 addition & 1 deletion 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
4 changes: 1 addition & 3 deletions updater/spec/dependabot/integration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
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
Expand Down Expand Up @@ -79,8 +79,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_ecosystem 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 c2c28d2

Please sign in to comment.