From eae468378e1a1abc7f84a76e7b0ab5b3227100aa Mon Sep 17 00:00:00 2001 From: Enda Phelan Date: Fri, 11 Dec 2020 10:50:45 +0000 Subject: [PATCH] refactor: better error handling --- pkg/cmd/kafka/topics/create/create.go | 6 ++--- pkg/cmd/kafka/topics/delete/delete.go | 2 +- pkg/cmd/kafka/topics/list/list.go | 4 +-- pkg/cmd/kafka/topics/topics.go | 2 -- pkg/sdk/kafka/topics/topics.go | 39 ++++++++++++++++----------- 5 files changed, 29 insertions(+), 24 deletions(-) diff --git a/pkg/cmd/kafka/topics/create/create.go b/pkg/cmd/kafka/topics/create/create.go index 9af7e59df8..d9b79d6bea 100644 --- a/pkg/cmd/kafka/topics/create/create.go +++ b/pkg/cmd/kafka/topics/create/create.go @@ -47,14 +47,14 @@ func createTopic(cmd *cobra.Command, _ []string) { } err := topics.ValidateCredentials() if err != nil { - fmt.Fprintf(os.Stderr, "Error creating credentials for topic: %v\n", topicName) + fmt.Fprintf(os.Stderr, "Error creating credentials for topic: %v\n", err) return } err = topics.CreateKafkaTopic(&topicConfigs) if err != nil { - fmt.Fprintf(os.Stderr, "Error creating topic: %v\n", topicName) + fmt.Fprintf(os.Stderr, "Error creating topic: %v\n", err) return } - fmt.Fprintf(os.Stderr, "Topic %v created\n", topicName) + fmt.Fprintf(os.Stderr, "Topic %v created\n", err) } diff --git a/pkg/cmd/kafka/topics/delete/delete.go b/pkg/cmd/kafka/topics/delete/delete.go index 09000f45d0..aa75897476 100644 --- a/pkg/cmd/kafka/topics/delete/delete.go +++ b/pkg/cmd/kafka/topics/delete/delete.go @@ -33,7 +33,7 @@ func deleteTopic(cmd *cobra.Command, _ []string) { fmt.Fprintf(os.Stderr, "Error creating credentials for topic: %v\n", topicName) return } - err := topics.DeleteKafkaTopic(topicName) + err = topics.DeleteKafkaTopic(topicName) if err != nil { fmt.Fprintf(os.Stderr, "Error deleting topic: %v\n", topicName) return diff --git a/pkg/cmd/kafka/topics/list/list.go b/pkg/cmd/kafka/topics/list/list.go index 8d0d1e96f5..129604dc36 100644 --- a/pkg/cmd/kafka/topics/list/list.go +++ b/pkg/cmd/kafka/topics/list/list.go @@ -30,11 +30,11 @@ func listTopic(cmd *cobra.Command, _ []string) { err := topics.ValidateCredentials() if err != nil { - fmt.Fprintf(os.Stderr, "Error creating credentials for list") + fmt.Fprintf(os.Stderr, "Error creating credentials for list: %v\n", err) return } err = topics.ListKafkaTopics() if err != nil { - fmt.Fprintf(os.Stderr, "Failed to perform list operation\n") + fmt.Fprintf(os.Stderr, "Failed to perform list operation: %v\n", err) } } diff --git a/pkg/cmd/kafka/topics/topics.go b/pkg/cmd/kafka/topics/topics.go index 02db1951ef..558eb5b6c1 100644 --- a/pkg/cmd/kafka/topics/topics.go +++ b/pkg/cmd/kafka/topics/topics.go @@ -6,7 +6,6 @@ import ( "github.com/bf2fc6cc711aee1a0c2a/cli/pkg/cmd/kafka/topics/create" "github.com/bf2fc6cc711aee1a0c2a/cli/pkg/cmd/kafka/topics/delete" "github.com/bf2fc6cc711aee1a0c2a/cli/pkg/cmd/kafka/topics/list" - "github.com/bf2fc6cc711aee1a0c2a/cli/pkg/cmd/kafka/topics/update" ) const ( @@ -25,7 +24,6 @@ func NewTopicsCommand() *cobra.Command { cmd.AddCommand( create.NewCreateTopicCommand(), list.NewListTopicCommand(), - update.NewUpdateTopicCommand(), delete.NewDeleteTopicCommand(), ) return cmd diff --git a/pkg/sdk/kafka/topics/topics.go b/pkg/sdk/kafka/topics/topics.go index f00d3edfe7..e0a3778f02 100644 --- a/pkg/sdk/kafka/topics/topics.go +++ b/pkg/sdk/kafka/topics/topics.go @@ -16,10 +16,10 @@ import ( "github.com/segmentio/kafka-go/sasl/plain" ) -func brokerConnect() (broker *kafka.Conn, ctl *kafka.Conn) { +func brokerConnect() (broker *kafka.Conn, ctl *kafka.Conn, err error) { cfg, err := config.Load() if err != nil { - fmt.Fprint(os.Stderr, err) + return nil, nil, err } mechanism := plain.Mechanism{ Username: cfg.ServiceAuth.ClientID, @@ -32,26 +32,24 @@ func brokerConnect() (broker *kafka.Conn, ctl *kafka.Conn) { SASLMechanism: mechanism, } - cfg, err := config.Load() + cfg, err = config.Load() if err != nil { - fmt.Fprint(os.Stderr, err) + return nil, nil, err } if cfg.Services.Kafka.ClusterID == "" { - fmt.Fprint(os.Stderr, "No Kafka selected. Run rhoas kafka use") - panic("Missing config") + return nil, nil, fmt.Errorf("No Kafka selected. Run rhoas kafka use") } connection, err := cfg.Connection() if err != nil { - fmt.Fprintf(os.Stderr, "Could not create connection: %v\n", err) + return nil, nil, fmt.Errorf("Could not create connection: %w", err) } managedservices := connection.NewMASClient() kafkaInstance, _, err := managedservices.DefaultApi.GetKafkaById(context.TODO(), cfg.Services.Kafka.ClusterID) if err != nil { - fmt.Fprintf(os.Stderr, "Could not get Kafka instance: %v\n", err) - return + return nil, nil, fmt.Errorf("Could not get Kafka instance: %w", err) } var clusterURL string @@ -63,20 +61,20 @@ func brokerConnect() (broker *kafka.Conn, ctl *kafka.Conn) { conn, err := dialer.Dial("tcp", clusterURL) if err != nil { - panic(err.Error()) + return nil, nil, err } controller, err := conn.Controller() if err != nil { - panic(err.Error()) + return nil, nil, err } controllerConn, err := kafka.Dial("tcp", net.JoinHostPort(controller.Host, strconv.Itoa(controller.Port))) if err != nil { - panic(err.Error()) + return nil, nil, err } - return conn, controllerConn + return conn, controllerConn, nil } func ValidateCredentials() error { @@ -109,7 +107,10 @@ func ValidateCredentials() error { } func CreateKafkaTopic(topicConfigs *[]kafka.TopicConfig) error { - conn, controllerConn := brokerConnect() + conn, controllerConn, err := brokerConnect() + if err != nil { + return err + } defer conn.Close() defer controllerConn.Close() @@ -118,7 +119,10 @@ func CreateKafkaTopic(topicConfigs *[]kafka.TopicConfig) error { } func DeleteKafkaTopic(topic string) error { - conn, controllerConn := brokerConnect() + conn, controllerConn, err := brokerConnect() + if err != nil { + return err + } defer conn.Close() defer controllerConn.Close() @@ -127,7 +131,10 @@ func DeleteKafkaTopic(topic string) error { } func ListKafkaTopics() error { - conn, controllerConn := brokerConnect() + conn, controllerConn, err := brokerConnect() + if err != nil { + return err + } defer conn.Close() defer controllerConn.Close()