Skip to content

refactor: improve list command performance#367

Merged
neurosnap merged 7 commits intomasterfrom
sc-30651-perf-list-cmds
Feb 10, 2025
Merged

refactor: improve list command performance#367
neurosnap merged 7 commits intomasterfrom
sc-30651-perf-list-cmds

Conversation

@neurosnap
Copy link
Contributor

@neurosnap neurosnap commented Jan 31, 2025

https://app.shortcut.com/aptible/story/30651/cli-performance-for-list-commands

environment:list

before

$ time bundle exec bin/aptible environment:list

________________________________________________________
Executed in    2.84 secs      fish           external
   usr time  426.77 millis    0.25 millis  426.53 millis
   sys time  139.25 millis    1.70 millis  137.55 millis

after

$ time bundle exec bin/aptible environment:list

________________________________________________________
Executed in    2.93 secs      fish           external
   usr time  383.18 millis    0.24 millis  382.94 millis
   sys time  142.15 millis    1.64 millis  140.51 millis

apps

before

$ time bundle exec bin/aptible apps

________________________________________________________
Executed in   14.55 secs    fish           external
   usr time    1.26 secs    0.26 millis    1.26 secs
   sys time    0.21 secs    1.70 millis    0.21 secs

after

$ time bundle exec bin/aptible apps

________________________________________________________
Executed in    5.86 secs      fish           external
   usr time  600.29 millis    0.23 millis  600.06 millis
   sys time  200.44 millis    1.54 millis  198.90 millis

db:list

before

$ time bundle exec bin/aptible db:list

________________________________________________________
Executed in   47.10 secs    fish           external
   usr time    2.00 secs    0.24 millis    2.00 secs
   sys time    0.38 secs    1.75 millis    0.38 secs

after

$ time bundle exec bin/aptible db:list

________________________________________________________
Executed in    4.86 secs      fish           external
   usr time  495.17 millis    0.22 millis  494.95 millis
   sys time  143.12 millis    1.48 millis  141.64 millis

@neurosnap neurosnap marked this pull request as ready for review February 3, 2025 19:35
Aptible::Resource.configure { |conf| conf.user_agent = version_string }
Aptible::Resource.configure do |conf|
conf.user_agent = version_string
# conf.logger.tap { |l| l.level = Logger::INFO }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is nice for debugging and I kind of want to keep it in the code so it's discoverable.

Copy link
Member

@UserNotFound UserNotFound left a comment

Choose a reason for hiding this comment

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

I haven't done any code review, but it seems to break listing apps for an environment aptible apps --env chaos

image

Happy to review more once the reason for that is found/fixed.

@neurosnap
Copy link
Contributor Author

Nice catch! I missed a couple of account checks, should be fixed now.

UserNotFound
UserNotFound previously approved these changes Feb 7, 2025
Copy link
Member

@UserNotFound UserNotFound left a comment

Choose a reason for hiding this comment

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

LGTM! This is much easier than the mess I got into looking at modifying aptible-resource or whatever...

One nit would be that if you intend for end-users to provide debug logging via the commented out conf.logger.tap { |l| l.level = Logger::INFO }, we should just make it toggled via an ENV. If it's just for us, code/comment is totally fine as is.

@neurosnap
Copy link
Contributor Author

Added an environment variable APTIBLE_DEBUG to set the log level for aptible-resource

@neurosnap neurosnap merged commit 80f3f47 into master Feb 10, 2025
11 checks passed
@neurosnap neurosnap deleted the sc-30651-perf-list-cmds branch February 10, 2025 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants