Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Commit

Permalink
Merge pull request #11 from intelsdi-x/fix_for_#10
Browse files Browse the repository at this point in the history 
Added better error handling when a DB driver is wrong defined (fix #10)
  • Loading branch information
@daria-lewandowska
daria-lewandowska authored Sep 26, 2016
2 parents efa40f9 + e45adbc commit 657ecea
Show file tree
Hide file tree
Showing 7 changed files with 209 additions and 158 deletions.
25 changes: 12 additions & 13 deletions dbi/connect.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package dbi
import (
"errors"
"fmt"
"os"

_ "github.com/go-sql-driver/mysql"
_ "github.com/lib/pq"
Expand All @@ -41,7 +40,6 @@ func getDefaultPort(driver string) string {

// openDB opens a database and verifies connection by calling ping to it
func openDB(db *dtype.Database) error {

var dsn string

// if port is not defined, set defaults
Expand All @@ -61,7 +59,6 @@ func openDB(db *dtype.Database) error {
default:
return fmt.Errorf("SQL Driver %s is not supported", db.Driver)
}

err := db.Executor.Open(db.Driver, dsn)
if err != nil {
return err
Expand Down Expand Up @@ -90,11 +87,8 @@ func openDBs(dbs map[string]*dtype.Database) error {
once := false
for i := range dbs {
err := openDB(dbs[i])

if err != nil {
closeDB(dbs[i])
fmt.Fprintf(os.Stderr, "Cannot open database `%+v`, err=%+v\n", dbs[i].DBName, err)
continue
return err
}
once = true
}
Expand All @@ -108,20 +102,25 @@ func openDBs(dbs map[string]*dtype.Database) error {

// closeDB closes a database
func closeDB(db *dtype.Database) error {
err := db.Executor.Close()
if err != nil {
return err
if db.Active {
err := db.Executor.Close()
if err != nil {
return err
}
db.Active = false
}
db.Active = false
return nil
}

// closeDBs closes databases (exported due to use in main.go)
func closeDBs(dbs map[string]*dtype.Database) {
func closeDBs(dbs map[string]*dtype.Database) []error {
//errors := []error{}
var errors []error
for i := range dbs {
err := closeDB(dbs[i])
if err != nil {
fmt.Fprintf(os.Stdout, "Cannot close connection to database %+v, err=%+v\n", dbs[i].DBName, err)
errors = append(errors, err)
}
}
return errors
}
18 changes: 9 additions & 9 deletions dbi/dbi.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package dbi

import (
"errors"
"fmt"
"os"
"strings"
Expand Down Expand Up @@ -62,15 +61,12 @@ func (dbiPlg *DbiPlugin) CollectMetrics(mts []plugin.MetricType) ([]plugin.Metri
// Cannot obtained sql settings
return nil, err
}

err = openDBs(dbiPlg.databases)
if err != nil {
return nil, err
}

dbiPlg.initialized = true
} // end of initialization

// execute dbs queries and get output
data, err = dbiPlg.executeQueries()
if err != nil {
Expand Down Expand Up @@ -153,7 +149,6 @@ func (dbiPlg *DbiPlugin) getMetrics() (map[string]interface{}, error) {
metrics := map[string]interface{}{}

err := openDBs(dbiPlg.databases)
defer closeDBs(dbiPlg.databases)

if err != nil {
return nil, err
Expand All @@ -165,6 +160,14 @@ func (dbiPlg *DbiPlugin) getMetrics() (map[string]interface{}, error) {
return nil, err
}

errors := closeDBs(dbiPlg.databases)
if errors != nil {
var dbs []string
for r := range errors {
dbs = append(dbs, errors[r].Error())
}
return metrics, fmt.Errorf("Cannot close database(s):\n %s", dbs)
}
return metrics, nil
}

Expand All @@ -175,7 +178,6 @@ func (dbiPlg *DbiPlugin) executeQueries() (map[string]interface{}, error) {

//execute queries for each defined databases
for dbName, db := range dbiPlg.databases {

if !db.Active {
//skip if db is not active (none established connection)
fmt.Fprintf(os.Stderr, "Cannot execute queries for database %s, is inactive (connection was not established properly)\n", dbName)
Expand All @@ -187,7 +189,6 @@ func (dbiPlg *DbiPlugin) executeQueries() (map[string]interface{}, error) {
statement := dbiPlg.queries[queryName].Statement

out, err := db.Executor.Query(queryName, statement)

if err != nil {
// log failing query and take the next one
fmt.Fprintf(os.Stderr, "Cannot execute query %s for database %s", queryName, dbName)
Expand All @@ -196,7 +197,6 @@ func (dbiPlg *DbiPlugin) executeQueries() (map[string]interface{}, error) {

for resName, res := range dbiPlg.queries[queryName].Results {
instanceOk := false

// to avoid inconsistency of columns names caused by capital letters (especially for postgresql driver)
instanceFrom := strings.ToLower(res.InstanceFrom)
valueFrom := strings.ToLower(res.ValueFrom)
Expand Down Expand Up @@ -227,7 +227,7 @@ func (dbiPlg *DbiPlugin) executeQueries() (map[string]interface{}, error) {
} // end of range databases

if len(data) == 0 {
return nil, errors.New("No data obtained from defined queries")
return nil, fmt.Errorf("No data obtained from defined queries")
}

return data, nil
Expand Down
77 changes: 29 additions & 48 deletions dbi/dbi_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,7 @@ func TestGetMetricTypes(t *testing.T) {
Convey("when path to setfile is incorrect", func() {
cfg := plugin.NewPluginConfigType()
dbiPlugin := New()
// mockdata.FileName has not existed yet
deleteMockFile()
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.FileName})
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: "./noFile.json"})
So(func() { dbiPlugin.GetMetricTypes(cfg) }, ShouldNotPanic)
results, err := dbiPlugin.GetMetricTypes(cfg)
So(err, ShouldNotBeNil)
Expand All @@ -127,12 +125,21 @@ func TestGetMetricTypes(t *testing.T) {
So(results, ShouldBeNil)
})

Convey("when there is wrong driver typed", func() {
cfg := plugin.NewPluginConfigType()
dbiPlugin := New()
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.SetfileIncorr})

So(func() { dbiPlugin.GetMetricTypes(cfg) }, ShouldNotPanic)
results, err := dbiPlugin.GetMetricTypes(cfg)
So(err, ShouldNotBeNil)
So(results, ShouldBeNil)
})

Convey("when cannot open db", func() {
cfg := plugin.NewPluginConfigType()
dbiPlugin := New()
createMockFile()
defer deleteMockFile()
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.FileName})
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.SetfileCorr})
mc := &mcMock{stmts: make(map[string]*sql.Stmt)}

//mockExecution outputs
Expand All @@ -154,9 +161,9 @@ func TestGetMetricTypes(t *testing.T) {
Convey("when cannot open, neither close db", func() {
cfg := plugin.NewPluginConfigType()
dbiPlugin := New()
createMockFile()
defer deleteMockFile()
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.FileName})
//createMockFile(correct)
//defer deleteMockFile()
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.SetfileCorr})
mc := &mcMock{stmts: make(map[string]*sql.Stmt)}

//mockExecution outputs
Expand All @@ -178,9 +185,7 @@ func TestGetMetricTypes(t *testing.T) {
Convey("when cannot ping the open db", func() {
cfg := plugin.NewPluginConfigType()
dbiPlugin := New()
createMockFile()
defer deleteMockFile()
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.FileName})
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.SetfileCorr})
mc := &mcMock{stmts: make(map[string]*sql.Stmt)}

//mockExecution outputs
Expand All @@ -202,9 +207,7 @@ func TestGetMetricTypes(t *testing.T) {
Convey("when cannot switch to selected db", func() {
cfg := plugin.NewPluginConfigType()
dbiPlugin := New()
createMockFile()
defer deleteMockFile()
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.FileName})
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.SetfileCorr})
mc := &mcMock{stmts: make(map[string]*sql.Stmt)}

//mockExecution outputs
Expand All @@ -226,9 +229,7 @@ func TestGetMetricTypes(t *testing.T) {
Convey("when execution of query returns error", func() {
cfg := plugin.NewPluginConfigType()
dbiPlugin := New()
createMockFile()
defer deleteMockFile()
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.FileName})
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.SetfileCorr})
mc := &mcMock{stmts: make(map[string]*sql.Stmt)}

//mockExecution outputs
Expand All @@ -250,9 +251,7 @@ func TestGetMetricTypes(t *testing.T) {
Convey("when query returns empty output", func() {
cfg := plugin.NewPluginConfigType()
dbiPlugin := New()
createMockFile()
defer deleteMockFile()
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.FileName})
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.SetfileCorr})

mc := &mcMock{stmts: make(map[string]*sql.Stmt)}

Expand All @@ -276,9 +275,7 @@ func TestGetMetricTypes(t *testing.T) {
Convey("successfully obtain metrics name", func() {
cfg := plugin.NewPluginConfigType()
dbiPlugin := New()
createMockFile()
defer deleteMockFile()
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.FileName})
cfg.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.SetfileCorr})

mc := &mcMock{stmts: make(map[string]*sql.Stmt)}

Expand Down Expand Up @@ -319,14 +316,13 @@ func TestCollectMetrics(t *testing.T) {

// set metrics config
config := cdata.NewNode()
config.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.FileName})
config.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.SetfileCorr})
for i, _ := range mts {
mts[i].Config_ = config
}

Convey("incorrect path to setfile", func() {
// mockdata.FileName has not existed (remove it just in case)
deleteMockFile()
config.AddItem("setfile", ctypes.ConfigValueStr{Value: "./noFile.json"})
dbiPlugin := New()
So(func() { dbiPlugin.CollectMetrics(mts) }, ShouldNotPanic)
results, err := dbiPlugin.CollectMetrics(mts)
Expand All @@ -338,6 +334,7 @@ func TestCollectMetrics(t *testing.T) {
dbiPlugin := New()
os.Create(mockdata.FileName)
defer os.Remove(mockdata.FileName)
config.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.FileName})
So(func() { dbiPlugin.CollectMetrics(mts) }, ShouldNotPanic)
results, err := dbiPlugin.CollectMetrics(mts)
So(err, ShouldNotBeNil)
Expand All @@ -350,14 +347,11 @@ func TestCollectMetrics(t *testing.T) {

mts := mockdata.Mts
config := cdata.NewNode()
config.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.FileName})
config.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.SetfileCorr})
for i, _ := range mts {
mts[i].Config_ = config
}

createMockFile()
defer deleteMockFile()

Convey("when cannot connect to databases", func() {
dbiPlugin := New()
mc := &mcMock{stmts: make(map[string]*sql.Stmt)}
Expand Down Expand Up @@ -402,8 +396,6 @@ func TestCollectMetrics(t *testing.T) {

Convey("when metric's value is a timestamp - special usecase", t, func() {
// to cover func fixDataType for time.Time case
createMockFile()
defer deleteMockFile()

dbiPlugin := New()
mc := &mcMock{stmts: make(map[string]*sql.Stmt)}
Expand All @@ -420,7 +412,7 @@ func TestCollectMetrics(t *testing.T) {

mts := mockdata.Mts
config := cdata.NewNode()
config.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.FileName})
config.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.SetfileCorr})
for i, _ := range mts {
mts[i].Config_ = config
}
Expand All @@ -432,8 +424,8 @@ func TestCollectMetrics(t *testing.T) {
})

Convey("collect metrics successfully", t, func() {
createMockFile()
defer deleteMockFile()
//createMockFile(correct)
//defer deleteMockFile()

dbiPlugin := New()
mc := &mcMock{stmts: make(map[string]*sql.Stmt)}
Expand All @@ -450,7 +442,7 @@ func TestCollectMetrics(t *testing.T) {

mts := mockdata.Mts
config := cdata.NewNode()
config.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.FileName})
config.AddItem("setfile", ctypes.ConfigValueStr{Value: mockdata.SetfileCorr})
mts[0].Config_ = config

So(func() { dbiPlugin.CollectMetrics(mts) }, ShouldNotPanic)
Expand All @@ -461,14 +453,3 @@ func TestCollectMetrics(t *testing.T) {
})

}

func createMockFile() {
deleteMockFile()

f, _ := os.Create(mockdata.FileName)
f.Write(mockdata.FileCont)
}

func deleteMockFile() {
os.Remove(mockdata.FileName)
}
Loading

0 comments on commit 657ecea

Please sign in to comment.