jprdonnelly/steampipe
1
0
Fork 0
mirror of https://github.com/turbot/steampipe.git synced 2025-12-19 18:12:43 -05:00
Code Issues Packages Projects Releases Wiki Activity

Rationalising 'status hooks'. Do not show spinner when setting status, instead require an explicit Show. #2432

Browse Source
This commit is contained in:
Binaek Sarkar
2023-03-14 17:18:14 +05:30
committed by GitHub
parent ec35487fd9
commit ee8b6647c4
18 changed files with 155 additions and 148 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
cmd/root.go
View File

@@ -370,7 +370,7 @@ func createRootContext() context.Context {
var statusRenderer statushooks.StatusHooks = statushooks.NullHooks
// if the client is a TTY, inject a status spinner
if isatty.IsTerminal(os.Stdout.Fd()) {
statusRenderer = newStatusSpinnerHook()
statusRenderer = statushooks.NewStatusSpinnerHook()
}
ctx := statushooks.AddStatusHooksToContext(context.Background(), statusRenderer)

113
cmd/service.go
View File

@@ -20,8 +20,11 @@ import (
"github.com/turbot/steampipe/pkg/display"
"github.com/turbot/steampipe/pkg/error_helpers"
"github.com/turbot/steampipe/pkg/statushooks"
"github.com/turbot/steampipe/pkg/steampipeconfig"
"github.com/turbot/steampipe/pkg/steampipeconfig/modconfig"
"github.com/turbot/steampipe/pkg/utils"
"github.com/turbot/steampipe/pluginmanager"
"github.com/turbot/steampipe/sperr"
)
func serviceCmd() *cobra.Command {
@@ -180,6 +183,20 @@ func runServiceStartCmd(cmd *cobra.Command, _ []string) {
error_helpers.FailOnError(invoker.IsValid())
}
startResult, dashboardState, dbServiceStarted := startService(ctx, port, serviceListen, invoker)
alreadyRunning := !dbServiceStarted
printStatus(ctx, startResult.DbState, startResult.PluginManagerState, dashboardState, alreadyRunning)
if viper.GetBool(constants.ArgForeground) {
runServiceInForeground(ctx)
}
}
func startService(ctx context.Context, port int, serviceListen db_local.StartListenType, invoker constants.Invoker) (_ *db_local.StartResult, _ *dashboardserver.DashboardServiceState, dbServiceStarted bool) {
statushooks.Show(ctx)
defer statushooks.Done(ctx)
err := db_local.EnsureDBInstalled(ctx)
if err != nil {
exitCode = constants.ExitCodeServiceStartupFailure
@@ -199,6 +216,7 @@ func runServiceStartCmd(cmd *cobra.Command, _ []string) {
return
}
// if the service is already running, then service start should make the service persistent
if startResult.Status == db_local.ServiceAlreadyRunning {
// check that we have the same port and listen parameters
@@ -221,34 +239,19 @@ func runServiceStartCmd(cmd *cobra.Command, _ []string) {
}
// if the service was started
if startResult.Status == db_local.ServiceStarted {
//
// this is required since RefreshConnectionAndSearchPaths may end up
// displaying warnings
//
// At the moment warnings is implemented in error_helpers.ShowWarning
// which does not have access to the working context and in effect the
// status spinner
//
// TODO: fix this
statushooks.Done(ctx)
muteCtx := statushooks.DisableStatusHooks(ctx)
// do
err = db_local.RefreshConnectionAndSearchPaths(muteCtx, invoker)
if err != nil {
_, err1 := db_local.StopServices(ctx, false, constants.InvokerService)
if err1 != nil {
error_helpers.ShowError(ctx, err1)
exitCode = constants.ExitCodeServiceSetupFailure
dbServiceStarted = startResult.Status == db_local.ServiceStarted
if dbServiceStarted {
refreshResult := refreshConnectionsWithLocalClient(ctx, invoker)
if refreshResult.GetError() != nil {
_, stopErr := db_local.StopServices(ctx, false, constants.InvokerService)
if stopErr != nil {
error_helpers.ShowError(ctx, sperr.WrapWithRootMessage(stopErr, "couldn't stop service after it was started"))
}
error_helpers.FailOnError(err)
exitCode = constants.ExitCodeServiceSetupFailure
error_helpers.FailOnError(refreshResult.GetError())
}
}
servicesStarted := startResult.Status == db_local.ServiceStarted
var dashboardState *dashboardserver.DashboardServiceState
if viper.GetBool(constants.ArgDashboard) {
dashboardState, err = dashboardserver.GetDashboardServiceState()
@@ -264,15 +267,10 @@ func runServiceStartCmd(cmd *cobra.Command, _ []string) {
tryToStopServices(ctx)
return
}
servicesStarted = true
dbServiceStarted = true
}
}
printStatus(ctx, startResult.DbState, startResult.PluginManagerState, dashboardState, !servicesStarted)
if viper.GetBool(constants.ArgForeground) {
runServiceInForeground(ctx)
}
return startResult, dashboardState, dbServiceStarted
}
func tryToStopServices(ctx context.Context) {
@@ -390,6 +388,17 @@ func runServiceRestartCmd(cmd *cobra.Command, _ []string) {
}
}()
dbStartResult, currentDashboardState := restartService(ctx)
if dbStartResult != nil {
printStatus(ctx, dbStartResult.DbState, dbStartResult.PluginManagerState, currentDashboardState, false)
}
}
func restartService(ctx context.Context) (_ *db_local.StartResult, _ *dashboardserver.DashboardServiceState) {
statushooks.Show(ctx)
defer statushooks.Done(ctx)
// get current db statue
currentDbState, err := db_local.GetState()
error_helpers.FailOnError(err)
@@ -448,22 +457,16 @@ to force a restart.
return
}
// this is required since RefreshConnectionAndSearchPaths may end up
// displaying warnings
//
// At the moment warnings is implemented in error_helpers.ShowWarning
// which does not have access to the working context and in effect the
// status spinner
//
// TODO: fix this
statushooks.Done(ctx)
muteCtx := statushooks.DisableStatusHooks(ctx)
// refresh connections
err = db_local.RefreshConnectionAndSearchPaths(muteCtx, constants.InvokerService)
if err != nil {
refreshResult := refreshConnectionsWithLocalClient(ctx, constants.InvokerService)
if refreshResult.GetError() != nil {
// we don't want to stop the service here, since this is a restart
// and the service has already been restarted
// the worst-case here is that we will end up with a service
// without refreshed connections - for which the error is shown
// at least we are not pulling the service out from under
exitCode = constants.ExitCodeServiceSetupFailure
error_helpers.FailOnError(err)
error_helpers.FailOnError(refreshResult.GetError())
}
// if the dashboard was running, start it
@@ -476,7 +479,7 @@ to force a restart.
error_helpers.FailOnError(err)
}
printStatus(ctx, dbStartResult.DbState, dbStartResult.PluginManagerState, currentDashboardState, false)
return dbStartResult, currentDashboardState
}
func runServiceStatusCmd(cmd *cobra.Command, _ []string) {
@@ -627,7 +630,6 @@ to force a shutdown.
`)
}
}
func showAllStatus(ctx context.Context) {
@@ -832,3 +834,20 @@ Not shutting down service as there as clients connected.
To force shutdown, press Ctrl+C again.
`
}
// refreshConnectionsWithLocalClient creates a local client and refreshed connections and search paths
func refreshConnectionsWithLocalClient(ctx context.Context, invoker constants.Invoker) *steampipeconfig.RefreshConnectionResult {
client, err := db_local.NewLocalClient(ctx, invoker, nil)
if err != nil {
return &steampipeconfig.RefreshConnectionResult{
ErrorAndWarnings: modconfig.ErrorAndWarnings{
Error: err,
},
}
}
defer client.Close(ctx)
statushooks.SetStatus(ctx, "Refreshing connections")
return client.RefreshConnectionAndSearchPaths(ctx)
}

6
pkg/constants/colors.go
View File

@@ -1,9 +1,15 @@
package constants
import (
"github.com/fatih/color"
"github.com/logrusorgru/aurora"
)
var (
ColoredErr = color.RedString("Error")
ColoredWarn = color.YellowString("Warning")
)
// Colors is a map of string to aurora colour value
var Colors = map[string]func(arg interface{}) aurora.Value{
"bold": Bold,

1
pkg/control/controlstatus/control_hooks_status.go
View File

@@ -27,6 +27,7 @@ func (c *StatusControlHooks) OnStart(ctx context.Context, _ *ControlProgress) {
}
statushooks.SetStatus(ctx, "Starting controls...")
statushooks.Show(ctx)
}
func (c *StatusControlHooks) OnControlStart(ctx context.Context, _ ControlRunStatusProvider, p *ControlProgress) {

18
pkg/dashboard/dashboardexecute/runtime_dependency_subscriber_impl.go
View File

@@ -3,29 +3,29 @@ package dashboardexecute
import (
"context"
"fmt"
"log"
"sync"
"github.com/turbot/go-kit/helpers"
typehelpers "github.com/turbot/go-kit/types"
"github.com/turbot/steampipe/pkg/dashboard/dashboardtypes"
"github.com/turbot/steampipe/pkg/error_helpers"
"github.com/turbot/steampipe/pkg/steampipeconfig/modconfig"
"golang.org/x/exp/maps"
"log"
"sync"
)
type RuntimeDependencySubscriberImpl struct {
// all RuntimeDependencySubscribers are also publishers as they have args/params
RuntimeDependencyPublisherImpl
// map of runtime dependencies, keyed by dependency long name
runtimeDependencies map[string]*dashboardtypes.ResolvedRuntimeDependency
// a list of the (scoped) names of any runtime dependencies that we rely on
RuntimeDependencyNames []string `json:"dependencies,omitempty"`
RawSQL string `json:"sql,omitempty"`
executeSQL string
// if the underlying resource has a base resource, create a RuntimeDependencySubscriberImpl instance to handle
// generation and publication of runtime depdencies from the base resource
baseDependencySubscriber *RuntimeDependencySubscriberImpl
// map of runtime dependencies, keyed by dependency long name
runtimeDependencies map[string]*dashboardtypes.ResolvedRuntimeDependency
RawSQL string `json:"sql,omitempty"`
executeSQL string
// a list of the (scoped) names of any runtime dependencies that we rely on
RuntimeDependencyNames []string `json:"dependencies,omitempty"`
}
func NewRuntimeDependencySubscriber(resource modconfig.DashboardLeafNode, parent dashboardtypes.DashboardParent, run dashboardtypes.DashboardTreeRun, executionTree *DashboardExecutionTree) *RuntimeDependencySubscriberImpl {

2
pkg/dashboard/dashboardexecute/snapshot.go
View File

@@ -3,13 +3,13 @@ package dashboardexecute
import (
"context"
"fmt"
"github.com/turbot/steampipe/pkg/steampipeconfig/modconfig"
"log"
"github.com/turbot/steampipe/pkg/dashboard/dashboardevents"
"github.com/turbot/steampipe/pkg/dashboard/dashboardtypes"
"github.com/turbot/steampipe/pkg/initialisation"
"github.com/turbot/steampipe/pkg/statushooks"
"github.com/turbot/steampipe/pkg/steampipeconfig/modconfig"
)
func GenerateSnapshot(ctx context.Context, target string, initData *initialisation.InitData, inputs map[string]any) (snapshot *dashboardtypes.SteampipeSnapshot, err error) {

3
pkg/db/db_client/db_client_connect.go
View File

@@ -29,6 +29,9 @@ func (c *DbClient) establishConnectionPool(ctx context.Context) error {
return err
}
// MinConns should default to 0, but when not set, it actually get very high values (e.g. 80217984)
// this leads to a huge number of connections getting created
// TODO BINAEK dig into this and figure out why this is happening.
// We need to be sure that it is not an issue with service management
config.MinConns = 0
config.MaxConns = int32(maxConnections)
config.MaxConnLifetime = connMaxLifetime

21
pkg/db/db_local/refresh_connections.go
View File

@@ -1,21 +0,0 @@
package db_local
import (
"context"
"github.com/turbot/steampipe/pkg/constants"
)
// RefreshConnectionAndSearchPaths creates a local client and refreshed connections and search paths
func RefreshConnectionAndSearchPaths(ctx context.Context, invoker constants.Invoker) error {
client, err := NewLocalClient(ctx, invoker, nil)
if err != nil {
return err
}
defer client.Close(ctx)
refreshResult := client.RefreshConnectionAndSearchPaths(ctx)
// display any initialisation warnings
refreshResult.ShowWarnings()
return refreshResult.Error
}

2
pkg/db/db_local/start_services.go
View File

@@ -26,9 +26,9 @@ import (
// StartResult is a pseudoEnum for outcomes of StartNewInstance
type StartResult struct {
Error error
Status StartDbStatus
DbState *RunningDBInstanceInfo
PluginManagerState *pluginmanager.PluginManagerState
Status StartDbStatus
}
func (r *StartResult) SetError(err error) *StartResult {

15
pkg/error_helpers/utils.go
View File

@@ -4,21 +4,16 @@ import (
"context"
"errors"
"fmt"
"github.com/spf13/viper"
"github.com/turbot/steampipe/pkg/constants"
"os"
"strings"
"github.com/fatih/color"
"github.com/shiena/ansicolor"
"github.com/spf13/viper"
"github.com/turbot/steampipe/pkg/constants"
"github.com/turbot/steampipe/pkg/statushooks"
)
var (
colorErr = color.RedString("Error")
colorWarn = color.YellowString("Warning")
)
func init() {
color.Output = ansicolor.NewAnsiColorWriter(os.Stderr)
}
@@ -50,7 +45,7 @@ func ShowError(ctx context.Context, err error) {
}
err = HandleCancelError(err)
statushooks.Done(ctx)
fmt.Fprintf(color.Output, "%s: %v\n", colorErr, TransformErrorToSteampipe(err))
fmt.Fprintf(color.Output, "%s: %v\n", constants.ColoredErr, TransformErrorToSteampipe(err))
}
// ShowErrorWithMessage displays the given error nicely with the given message
@@ -60,7 +55,7 @@ func ShowErrorWithMessage(ctx context.Context, err error, message string) {
}
err = HandleCancelError(err)
statushooks.Done(ctx)
fmt.Fprintf(color.Output, "%s: %s - %v\n", colorErr, message, TransformErrorToSteampipe(err))
fmt.Fprintf(color.Output, "%s: %s - %v\n", constants.ColoredErr, message, TransformErrorToSteampipe(err))
}
// TransformErrorToSteampipe removes the pq: and rpc error prefixes along
@@ -113,7 +108,7 @@ func ShowWarning(warning string) {
if len(warning) == 0 {
return
}
fmt.Fprintf(color.Output, "%s: %v\n", colorWarn, warning)
fmt.Fprintf(color.Output, "%s: %v\n", constants.ColoredWarn, warning)
}
func CombineErrorsWithPrefix(prefix string, errors ...error) error {

8
pkg/interactive/interactive_client.go
View File

@@ -428,12 +428,8 @@ func (c *InteractiveClient) getQuery(ctx context.Context, line string) *modconfi
c.cancelActiveQueryIfAny()
}()
// show the current status of initialization in the spinner
statushooks.SetStatus(ctx, c.initData.Status)
// set up initdata so that we can update the spinner when the status changes
c.initData.OnStatusChanged = func(newStatus string) {
statushooks.SetStatus(ctx, newStatus)
}
// show the spinner here while we wait for initialization to complete
statushooks.Show(ctx)
// wait for client initialisation to complete
err := c.waitForInitData(queryCtx)
statushooks.Done(ctx)

21
pkg/interactive/interactive_client_init.go
View File

@@ -54,9 +54,26 @@ func (c *InteractiveClient) handleInitResult(ctx context.Context, initResult *db
initResult.DisplayMessages()
// show the prompt again
c.hidePrompt = false
// We need to render the prompt here to make sure that it comes back
// after the messages have been displayed
c.interactivePrompt.Render()
// after the messages have been displayed (only if there's no execution)
//
// We check for query execution by TRYING to acquire the same lock that
// execution locks on
//
// If we can acquire a lock, that means that there's no
// query execution underway - and it is safe to render the prompt
//
// otherwise, that query execution is waiting for this init to finish
// and as such will be out of the prompt - in which case, we shouldn't
// re-render the prompt
//
// the prompt will be re-rendered when the query execution finished
if c.executionLock.TryLock() {
c.interactivePrompt.Render()
// release the lock
c.executionLock.Unlock()
}
}
// initialise autocomplete suggestions

19
pkg/query/init_data.go
View File

@@ -3,6 +3,7 @@ package query
import (
"context"
"fmt"
"github.com/spf13/viper"
"github.com/turbot/steampipe/pkg/constants"
"github.com/turbot/steampipe/pkg/export"
@@ -15,11 +16,6 @@ import (
type InitData struct {
initialisation.InitData
// the current state that init is in
Status string
// if non-nil, this is called everytime the status changes
OnStatusChanged func(string)
cancelInitialisation context.CancelFunc
Loaded chan struct{}
// map of query name to resolved query (key is the query text for command line queries)
@@ -39,13 +35,6 @@ func NewInitData(ctx context.Context, args []string) *InitData {
return i
}
func (i *InitData) SetStatus(newStatus string) {
i.Status = newStatus
if i.OnStatusChanged != nil {
i.OnStatusChanged(newStatus)
}
}
func queryExporters() []export.Exporter {
return []export.Exporter{&export.SnapshotExporter{}}
}
@@ -77,17 +66,13 @@ func (i *InitData) Cleanup(ctx context.Context) {
}
}
func (i *InitData) init(parentCtx context.Context, args []string) {
func (i *InitData) init(ctx context.Context, args []string) {
defer func() {
close(i.Loaded)
// clear the cancelInitialisation function
i.cancelInitialisation = nil
}()
// create a context with the init hook in - which can be sent down to lower level operations
hook := NewQueryInitStatusHook(i)
ctx := statushooks.AddStatusHooksToContext(parentCtx, hook)
// validate export args
if len(viper.GetStringSlice(constants.ArgExport)) > 0 {
i.RegisterExporters(queryExporters()...)

17
pkg/query/init_hook.go
View File

@@ -1,17 +0,0 @@
package query
type QueryInitStatusHook struct {
initData *InitData
}
func NewQueryInitStatusHook(initData *InitData) *QueryInitStatusHook {
hooks := new(QueryInitStatusHook)
hooks.initData = initData
return hooks
}
func (h *QueryInitStatusHook) SetStatus(status string) {
h.initData.SetStatus(status)
}
func (h *QueryInitStatusHook) Done() {}
func (h *QueryInitStatusHook) Message(...string) {}

10
pkg/statushooks/context.go
View File

@@ -56,7 +56,15 @@ func SetStatus(ctx context.Context, msg string) {
}
func Done(ctx context.Context) {
StatusHooksFromContext(ctx).Done()
StatusHooksFromContext(ctx).Hide()
}
func Warn(ctx context.Context, warning string) {
StatusHooksFromContext(ctx).Warn(warning)
}
func Show(ctx context.Context) {
StatusHooksFromContext(ctx).Show()
}
func Message(ctx context.Context, msgs ...string) {

4
pkg/statushooks/null_hooks.go
View File

@@ -5,5 +5,7 @@ var NullHooks = &NullStatusHook{}
type NullStatusHook struct{}
func (*NullStatusHook) SetStatus(string) {}
func (*NullStatusHook) Done() {}
func (*NullStatusHook) Hide() {}
func (*NullStatusHook) Message(...string) {}
func (*NullStatusHook) Show() {}
func (*NullStatusHook) Warn(string) {}

37
cmd/spinner.go → pkg/statushooks/spinner.go
View File

@@ -1,4 +1,4 @@
package cmd
package statushooks
import (
"fmt"
@@ -7,7 +7,9 @@ import (
"time"
"github.com/briandowns/spinner"
"github.com/fatih/color"
"github.com/karrick/gows"
"github.com/turbot/steampipe/pkg/constants"
)
// spinner format:
@@ -48,7 +50,7 @@ func WithDelay(delay time.Duration) StatusSpinnerOpt {
// We should never create a StatusSpinner directly. To use a spinner
// DO NOT use a StatusSpinner directly, since using it may have
// unintended side-effect around the spinner lifecycle
func newStatusSpinnerHook(opts ...StatusSpinnerOpt) *StatusSpinner {
func NewStatusSpinnerHook(opts ...StatusSpinnerOpt) *StatusSpinner {
res := &StatusSpinner{}
res.spinner = spinner.New(
@@ -67,9 +69,6 @@ func newStatusSpinnerHook(opts ...StatusSpinnerOpt) *StatusSpinner {
// SetStatus implements StatusHooks
func (s *StatusSpinner) SetStatus(msg string) {
s.UpdateSpinnerMessage(msg)
if !s.spinner.Active() {
s.startSpinner()
}
}
func (s *StatusSpinner) Message(msgs ...string) {
@@ -82,14 +81,32 @@ func (s *StatusSpinner) Message(msgs ...string) {
}
}
// Done implements StatusHooks
func (s *StatusSpinner) Done() {
func (s *StatusSpinner) Warn(msg string) {
if s.spinner.Active() {
s.spinner.Stop()
defer s.spinner.Start()
}
fmt.Fprintf(color.Output, "%s: %v\n", constants.ColoredWarn, msg)
}
// Hide implements StatusHooks
func (s *StatusSpinner) Hide() {
if s.cancel != nil {
close(s.cancel)
}
s.closeSpinner()
}
func (s *StatusSpinner) Show() {
s.spinner.Start()
}
// UpdateSpinnerMessage updates the message of the given spinner
func (s *StatusSpinner) UpdateSpinnerMessage(newMessage string) {
newMessage = s.truncateSpinnerMessageToScreen(newMessage)
s.spinner.Suffix = fmt.Sprintf(" %s", newMessage)
}
func (s *StatusSpinner) startSpinner() {
if s.cancel != nil {
// if there is a cancel channel, we are already waiting for the service to start after a delay
@@ -112,12 +129,6 @@ func (s *StatusSpinner) startSpinner() {
}()
}
// UpdateSpinnerMessage updates the message of the given spinner
func (s *StatusSpinner) UpdateSpinnerMessage(newMessage string) {
newMessage = s.truncateSpinnerMessageToScreen(newMessage)
s.spinner.Suffix = fmt.Sprintf(" %s", newMessage)
}
func (s *StatusSpinner) closeSpinner() {
if s.spinner != nil {
s.spinner.Stop()

4
pkg/statushooks/status_hooks.go
View File

@@ -2,6 +2,8 @@ package statushooks
type StatusHooks interface {
SetStatus(string)
Done()
Show()
Warn(string)
Hide()
Message(...string)
}