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

refactor: improve list command performance #367

Merged
merged 7 commits into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/aptible/cli/agent.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require 'base64'
require 'uri'
require 'logger'

require 'aptible/auth'
require 'thor'
Expand Down Expand Up @@ -80,7 +81,13 @@ def self.exit_on_failure?

def initialize(*)
nag_toolbelt unless toolbelt?
Aptible::Resource.configure { |conf| conf.user_agent = version_string }
Aptible::Resource.configure do |conf|
conf.user_agent = version_string
level = Logger::WARN
debug_level = ENV['APTIBLE_DEBUG']
level = debug_level if debug_level
conf.logger.tap { |l| l.level = level }
end
warn_sso_enforcement
super
end
Expand Down
17 changes: 16 additions & 1 deletion lib/aptible/cli/helpers/app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,27 @@ def ensure_service(options, type)
service
end

def apps_href
href = '/apps'
if Renderer.format != 'json'
href = '/apps?per_page=5000&no_embed=true'
end
href
end

def apps_all
Aptible::Api::App.all(
token: fetch_token,
href: apps_href
)
end

def apps_from_handle(handle, environment)
# TODO: This should probably use each_app for more efficiency.
if environment
environment.apps
else
Aptible::Api::App.all(token: fetch_token)
apps_all
end.select { |a| a.handle == handle }
end

Expand Down
25 changes: 20 additions & 5 deletions lib/aptible/cli/helpers/database.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,27 @@ def ensure_database(options = {})
end
end

def databases_from_handle(handle, environment)
if environment
databases = environment.databases
else
databases = Aptible::Api::Database.all(token: fetch_token)
def databases_href
href = '/databases'
if Renderer.format != 'json'
href = '/databases?per_page=5000&no_embed=true'
end
href
end

def databases_all
Aptible::Api::Database.all(
token: fetch_token,
href: databases_href
)
end

def databases_from_handle(handle, environment)
databases = if environment
environment.databases
else
databases_all
end
databases.select { |a| a.handle == handle }
end

Expand Down
31 changes: 28 additions & 3 deletions lib/aptible/cli/helpers/environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ module Helpers
module Environment
include Helpers::Token

def environment_href
href = '/accounts'
if Renderer.format != 'json'
href = '/accounts?per_page=5000&no_embed=true'
end
href
end

def scoped_environments(options)
if options[:environment]
if (environment = environment_from_handle(options[:environment]))
Expand All @@ -14,7 +22,11 @@ def scoped_environments(options)
raise Thor::Error, 'Specified account does not exist'
end
else
Aptible::Api::Account.all(token: fetch_token)
href = environment_href
Aptible::Api::Account.all(
token: fetch_token,
href: href
)
end
end

Expand All @@ -30,13 +42,26 @@ def ensure_environment(options = {})

def environment_from_handle(handle)
return nil unless handle
Aptible::Api::Account.all(token: fetch_token).find do |a|
href = environment_href
Aptible::Api::Account.all(token: fetch_token, href: href).find do |a|
a.handle == handle
end
end

def environment_map(accounts)
acc_map = {}
accounts.each do |account|
acc_map[account.links.self.href] = account
end
acc_map
end

def ensure_default_environment
environments = Aptible::Api::Account.all(token: fetch_token)
href = environment_href
environments = Aptible::Api::Account.all(
token: fetch_token,
href: href
)
case environments.count
when 0
e = 'No environments. Go to https://app.aptible.com/ to proceed'
Expand Down
6 changes: 5 additions & 1 deletion lib/aptible/cli/renderer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,12 @@ module CLI
module Renderer
FORMAT_VAR = 'APTIBLE_OUTPUT_FORMAT'.freeze

def self.format
ENV[FORMAT_VAR]
end

def self.current
case (format = ENV[FORMAT_VAR])
case format
when 'json'
Json.new
when 'text'
Expand Down
7 changes: 7 additions & 0 deletions lib/aptible/cli/resource_formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ def inject_app(node, app, account)
attach_account(node, account)
end

def inject_database_minimal(node, database, account)
node.value('id', database.id)
node.value('handle', database.handle)
node.value('created_at', database.created_at)
attach_account(node, account)
end

def inject_database(node, database, account)
node.value('id', database.id)
node.value('handle', database.handle)
Expand Down
18 changes: 16 additions & 2 deletions lib/aptible/cli/subcommands/apps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,22 @@ def apps
{ 'environment' => 'handle' },
'handle'
) do |node|
scoped_environments(options).each do |account|
account.each_app do |app|
accounts = scoped_environments(options)
acc_map = environment_map(accounts)

if Renderer.format == 'json'
accounts.each do |account|
account.each_app do |app|
node.object do |n|
ResourceFormatter.inject_app(n, app, account)
end
end
end
else
apps_all.each do |app|
account = acc_map[app.links.account.href]
next if account.nil?

node.object do |n|
ResourceFormatter.inject_app(n, app, account)
end
Expand Down
24 changes: 21 additions & 3 deletions lib/aptible/cli/subcommands/db.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,28 @@ def self.included(thor)
{ 'environment' => 'handle' },
'handle'
) do |node|
scoped_environments(options).each do |account|
account.each_database do |db|
accounts = scoped_environments(options)
acc_map = environment_map(accounts)

if Renderer.format == 'json'
accounts.each do |account|
account.each_database do |db|
node.object do |n|
ResourceFormatter.inject_database(n, db, account)
end
end
end
else
databases_all.each do |db|
account = acc_map[db.links.account.href]
next if account.nil?

node.object do |n|
ResourceFormatter.inject_database(n, db, account)
ResourceFormatter.inject_database_minimal(
n,
db,
account
)
end
end
end
Expand Down
17 changes: 12 additions & 5 deletions lib/aptible/cli/subcommands/log_drain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,18 @@ def self.drain_options
{ 'environment' => 'handle' },
'handle'
) do |node|
scoped_environments(options).each do |account|
account.log_drains.each do |drain|
node.object do |n|
ResourceFormatter.inject_log_drain(n, drain, account)
end
accounts = scoped_environments(options)
acc_map = environment_map(accounts)

Aptible::Api::LogDrain.all(
token: fetch_token,
href: '/log_drains?per_page=5000'
).each do |drain|
account = acc_map[drain.links.account.href]
next if account.nil?

node.object do |n|
ResourceFormatter.inject_log_drain(n, drain, account)
end
end
end
Expand Down
17 changes: 12 additions & 5 deletions lib/aptible/cli/subcommands/metric_drain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,18 @@ def self.included(thor)
{ 'environment' => 'handle' },
'handle'
) do |node|
scoped_environments(options).each do |account|
account.metric_drains.each do |drain|
node.object do |n|
ResourceFormatter.inject_metric_drain(n, drain, account)
end
accounts = scoped_environments(options)
acc_map = environment_map(accounts)

Aptible::Api::MetricDrain.all(
token: fetch_token,
href: '/metric_drains?per_page=5000'
).each do |drain|
account = acc_map[drain.links.account.href]
next if account.nil?

node.object do |n|
ResourceFormatter.inject_metric_drain(n, drain, account)
end
end
end
Expand Down
14 changes: 13 additions & 1 deletion spec/aptible/cli/subcommands/apps_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ def explain
describe '#apps' do
it 'lists an app in an account' do
allow(Aptible::Api::Account).to receive(:all).and_return([account])
allow(Aptible::Api::App)
.to receive(:all)
.and_return([app])
subject.send('apps')

expect(captured_output_text)
Expand All @@ -40,8 +43,11 @@ def explain
it 'lists multiple apps in an account' do
allow(Aptible::Api::Account).to receive(:all).and_return([account])
app2 = Fabricate(:app, handle: 'foobar', account: account)
subject.send('apps')
allow(Aptible::Api::App)
.to receive(:all)
.and_return([app, app2])

subject.send('apps')
expect(captured_output_text)
.to eq("=== #{account.handle}\n#{app.handle}\n#{app2.handle}\n")
end
Expand All @@ -54,6 +60,9 @@ def explain
app21 = Fabricate(:app, account: account2, handle: 'app21')
app22 = Fabricate(:app, account: account2, handle: 'app21')

allow(Aptible::Api::App)
.to receive(:all)
.and_return([app11, app21, app22])
allow(Aptible::Api::Account).to receive(:all)
.and_return([account1, account2])

Expand All @@ -77,6 +86,7 @@ def explain
app2 = Fabricate(:app, account: account2, handle: 'app2')
allow(subject).to receive(:options)
.and_return(environment: account2.handle)
allow(Aptible::Api::App).to receive(:all).and_return([app2])

allow(Aptible::Api::Account).to receive(:all)
.and_return([account, account2])
Expand All @@ -90,6 +100,7 @@ def explain
account = Fabricate(:account, handle: 'account')
app = Fabricate(:app, account: account, handle: 'app')
allow(Aptible::Api::Account).to receive(:all).and_return([account])
allow(Aptible::Api::App).to receive(:all).and_return([app])

s1 = Fabricate(
:service,
Expand Down Expand Up @@ -148,6 +159,7 @@ def explain
app = Fabricate(:app, account: account, handle: 'app',
last_deploy_operation: op)
allow(Aptible::Api::Account).to receive(:all).and_return([account])
allow(Aptible::Api::App).to receive(:all).and_return([app])

expected_json = [
{
Expand Down
6 changes: 4 additions & 2 deletions spec/aptible/cli/subcommands/config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@

before do
allow(Aptible::Api::App).to receive(:all)
.with(token: token).and_return([app])
.with(token: token, href: '/apps?per_page=5000&no_embed=true')
.and_return([app])
allow(Aptible::Api::Account).to receive(:all)
.with(token: token).and_return([account])
.with(token: token, href: '/apps?per_page=5000&no_embed=true')
.and_return([account])
end

before { allow(subject).to receive(:options) { { app: app.handle } } }
Expand Down
8 changes: 7 additions & 1 deletion spec/aptible/cli/subcommands/db_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -269,19 +269,25 @@ def expect_provision_database(create_opts, provision_opts = {})
staging = Fabricate(:account, handle: 'staging')
prod = Fabricate(:account, handle: 'production')

dbs = []
[
[staging, 'staging-redis-db'],
[staging, 'staging-postgres-db'],
[prod, 'prod-elsearch-db'],
[prod, 'prod-postgres-db']
].each do |a, h|
d = Fabricate(:database, account: a, handle: h)
dbs << d
Fabricate(:database_credential, database: d)
end

token = 'the-token'
allow(subject).to receive(:fetch_token) { token }
allow(Aptible::Api::Account).to receive(:all).with(token: token)
allow(Aptible::Api::Database).to receive(:all)
.with(token: token, href: '/databases?per_page=5000&no_embed=true')
.and_return(dbs)
allow(Aptible::Api::Account).to receive(:all)
.with(token: token, href: '/accounts?per_page=5000&no_embed=true')
.and_return([staging, prod])
end

Expand Down
Loading