feat: Avoid cli setup checks when not required #68

tfelix · 2021-07-20T08:41:31Z

In order to optimize startup time, especially when only issuing a non
cloud-cli required command, collie now only checks cloud cli configuration
when required.

Should mitigate or at least lessens @JohannesRudolph startup time issue.

In order to optimize startup time, especially when only issuing a non cloud-cli required command, collie now only checks cloud cli configuration when required.

JohannesRudolph · 2021-07-20T11:35:19Z

Thanks for taking this up so quickly. I think this is a step in the right direction and certainly fixes the use cases I described.

However I wonder if we can go even further. Some ideas:

make an "optimistic" assumption that the CLIs are installed when they are also configured (test cli is installed when running collie configure, when it fails with another command fall back on the standard error handling for CLI invocations)
run the checks concurrently (capture the promises returned by checkCli then await Promise.all(...). For example, gcloud --version takes ~1s on my machine and it's easy to see why invoking all three CLIs serially takes so long.

Jelledb · 2021-07-20T12:14:58Z

Thanks for taking this up so quickly. I think this is a step in the right direction and certainly fixes the use cases I described.

However I wonder if we can go even further. Some ideas:

make an "optimistic" assumption that the CLIs are installed when they are also configured (test cli is installed when running collie configure, when it fails with another command fall back on the standard error handling for CLI invocations)

run the checks concurrently (capture the promises returned by checkCli then await Promise.all(...). For example, gcloud --version takes ~1s on my machine and it's easy to see why invoking all three CLIs serially takes so long.

sounds like a bigger refactoring. Could you create a ticket to refactor this in the future?
sounds like a quick win. @tfelix could you pick this up in this PR?

not awaiting the logger configuration can cause loosing logs

see discussion in #68

github-actions · 2021-07-20T15:30:01Z

Error in deno fmt. Please check the Action logs.

JohannesRudolph · 2021-07-20T15:30:04Z

sounds like a quick win. @tfelix could you pick this up in this PR?

I took this up as I was also trying to see if the debug output showed me the sequential invocations - and fixed some issues in the logging as part of the setup.

tfelix · 2021-07-21T09:15:10Z

Thanks Johannes I merge this now then.

* fix: Collie config no longer crashes (#50) * fix: links for badges (#59) * fix: `collie cache` will show help (#58) * feat: Add tutorial links directly in README. (#52) * feat: Add images Wiki (#63) * feat: Rename totalUsageCost, fix issue with cached tenants, fix costs table missing data (#64) * Renamed `totalUsageCost` to `cost` so it takes less space * Fixed an issue with retrieving cached tenants. * Fixed table only showing one month per tenant at maximum (closes #51) * Update README.md Links were pointing to the wrong Wiki sections * fix: Log statements not hidden when piping collie output (#62) Now properly disables log output breaking the output when collie stdout is piped into a file or different command. * Add Website to README (#69) * feat: Avoid cli setup checks when not required (#68) * feat: Avoid cli setup checks when not required In order to optimize startup time, especially when only issuing a non cloud-cli required command, collie now only checks cloud cli configuration when required. * fix: setup logger before running commands and await it not awaiting the logger configuration can cause loosing logs * feat: run cli checks concurrently see discussion in #68 * feat: add debug logs to ShellRunner for every command * style: deno fmt Co-authored-by: Johannes Rudolph <jrudolph@meshcloud.io> * feat: Improved JSON parsing so it always logs a message, including the command. (#57) * feat: harmonize error message when cli not authenticated (#70) * Feature/issue 56 (#71) * feat: detect installed cli versions match expectations if not, give installation hints via collie readme. fixes #56 * feat: Update version number (#73) Co-authored-by: Dennis Murtic <73823855+dmurtic@users.noreply.github.com> Co-authored-by: Thomas Felix <1186998+tfelix@users.noreply.github.com> Co-authored-by: Johannes Rudolph <jrudolph@meshcloud.io>

* feat: Avoid cli setup checks when not required In order to optimize startup time, especially when only issuing a non cloud-cli required command, collie now only checks cloud cli configuration when required. * fix: setup logger before running commands and await it not awaiting the logger configuration can cause loosing logs * feat: run cli checks concurrently see discussion in #68 * feat: add debug logs to ShellRunner for every command * style: deno fmt Co-authored-by: Johannes Rudolph <jrudolph@meshcloud.io>

* feat: Add tutorial links directly in README. (#52) * feat: Add tutorial links directly in README. * And removed GitOps section, which is not supported yet. * Update README.md * Update README.md Added a different caption * feat: Avoid cli setup checks when not required (#68) * feat: Avoid cli setup checks when not required In order to optimize startup time, especially when only issuing a non cloud-cli required command, collie now only checks cloud cli configuration when required. * fix: setup logger before running commands and await it not awaiting the logger configuration can cause loosing logs * feat: run cli checks concurrently see discussion in #68 * feat: add debug logs to ShellRunner for every command * style: deno fmt Co-authored-by: Johannes Rudolph <jrudolph@meshcloud.io> * Feature/issue 56 (#71) * feat: detect installed cli versions match expectations if not, give installation hints via collie readme. fixes #56 * fix: handle GCP projects with empty IAM policies (#80) fixes #67 * fix: use azure subscription id instead of name to prevent word-splitting (#79) fixes #76 * Feature/cache statistics (#78) * refactor: generate entries summary generically for all platforms * feat: add query statistics to tty table output This lets users know whether results are served from cache or if they're queried from the cloud. At the moment only "tenant list" command supports this. Fixes #72 * feat: add query statistics to all tenant commands Fixes #72 * refactor: extract QueryStatistics class into its own module * test: add a tests for query statistics * feat: slightly tweak the query stats message. * feat: correctly print cache and cloud query statistics for hybrid query hybrid queries can occur when e.g. running - collie tenant list -> partially fills the tenant cache - collie tenant iam -> uses tenants from cache, adds IAM info * refactor: Uses OOP patterns Simplifies the statistics decorator to be less intrusive. Co-authored-by: Thomas Felix <tfelix@meshcloud.io> * Update issue templates * Update issue templates * fix: Spinner animation overwrites error message (#85) Improves the spinner animation termination to avoid part of the error message writing into the spinner animation text. * feat: Tags are shown in table cost view (#90) * fix: Fix costs / IAM missing in cached tenants. (#91) * Feature/add windows support (#87) * feat: add windows support for collie * feat: add windows binary to releases * fix: differentiation in posix and win32 path * fix: Replaces var with let * Fixed issue found while testing Windows * Added global `isWindows` var for consistency Co-authored-by: Thomas Felix <tfelix@meshcloud.io> Co-authored-by: Jelle den Burger <jdburger@meshcloud.io> * Bump version to 0.5.0 Co-authored-by: Thomas Felix <1186998+tfelix@users.noreply.github.com> Co-authored-by: Johannes Rudolph <jrudolph@meshcloud.io> Co-authored-by: Thomas Felix <tfelix@meshcloud.io> Co-authored-by: Dennis Murtic <73823855+dmurtic@users.noreply.github.com>

feat: Avoid cli setup checks when not required

b4ee4aa

In order to optimize startup time, especially when only issuing a non cloud-cli required command, collie now only checks cloud cli configuration when required.

tfelix requested review from Jelledb and dmurtic July 20, 2021 08:41

github-actions bot added the feature label Jul 20, 2021

tfelix linked an issue Jul 20, 2021 that may be closed by this pull request

collie [command] --help invocations are slow #65

Closed

Jelledb approved these changes Jul 20, 2021

View reviewed changes

JohannesRudolph added 3 commits July 20, 2021 17:23

fix: setup logger before running commands and await it

b65b72f

not awaiting the logger configuration can cause loosing logs

feat: run cli checks concurrently

f01d260

see discussion in #68

feat: add debug logs to ShellRunner for every command

47e7d2e

style: deno fmt

28e0f06

JohannesRudolph mentioned this pull request Jul 20, 2021

Feature/issue 56 #71

Merged

JohannesRudolph requested a review from Jelledb July 20, 2021 16:23

tfelix merged commit c5c49ef into develop Jul 21, 2021

tfelix deleted the feature/improve-startup-time branch July 21, 2021 09:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Avoid cli setup checks when not required #68

feat: Avoid cli setup checks when not required #68

Uh oh!

tfelix commented Jul 20, 2021

Uh oh!

JohannesRudolph commented Jul 20, 2021

Uh oh!

Jelledb commented Jul 20, 2021 •

edited

Loading

Uh oh!

github-actions bot commented Jul 20, 2021

Uh oh!

JohannesRudolph commented Jul 20, 2021 •

edited

Loading

Uh oh!

tfelix commented Jul 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat: Avoid cli setup checks when not required #68

feat: Avoid cli setup checks when not required #68

Uh oh!

Conversation

tfelix commented Jul 20, 2021

Uh oh!

JohannesRudolph commented Jul 20, 2021

Uh oh!

Jelledb commented Jul 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 20, 2021

Uh oh!

JohannesRudolph commented Jul 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tfelix commented Jul 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Jelledb commented Jul 20, 2021 •

edited

Loading

JohannesRudolph commented Jul 20, 2021 •

edited

Loading