Skip to content

Commit

Permalink
make not found err messages more consistent
Browse files Browse the repository at this point in the history
  • Loading branch information
dgnorton authored and corylanou committed Dec 3, 2015
1 parent 343f00d commit 657877d
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 33 deletions.
35 changes: 18 additions & 17 deletions meta/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"time"

"github.com/gogo/protobuf/proto"
"github.com/influxdb/influxdb"
"github.com/influxdb/influxdb/influxql"
"github.com/influxdb/influxdb/meta/internal"
)
Expand Down Expand Up @@ -174,14 +175,14 @@ func (data *Data) DropDatabase(name string) error {
return nil
}
}
return ErrDatabaseNotFound
return influxdb.ErrDatabaseNotFound(name)
}

// RetentionPolicy returns a retention policy for a database by name.
func (data *Data) RetentionPolicy(database, name string) (*RetentionPolicyInfo, error) {
di := data.Database(database)
if di == nil {
return nil, ErrDatabaseNotFound
return nil, influxdb.ErrDatabaseNotFound(database)
}

for i := range di.RetentionPolicies {
Expand All @@ -205,7 +206,7 @@ func (data *Data) CreateRetentionPolicy(database string, rpi *RetentionPolicyInf
// Find database.
di := data.Database(database)
if di == nil {
return ErrDatabaseNotFound
return influxdb.ErrDatabaseNotFound(database)
} else if di.RetentionPolicy(rpi.Name) != nil {
return ErrRetentionPolicyExists
}
Expand All @@ -226,7 +227,7 @@ func (data *Data) DropRetentionPolicy(database, name string) error {
// Find database.
di := data.Database(database)
if di == nil {
return ErrDatabaseNotFound
return influxdb.ErrDatabaseNotFound(database)
}

// Prohibit dropping the default retention policy.
Expand All @@ -241,21 +242,21 @@ func (data *Data) DropRetentionPolicy(database, name string) error {
return nil
}
}
return ErrRetentionPolicyNotFound
return influxdb.ErrRetentionPolicyNotFound(name)
}

// UpdateRetentionPolicy updates an existing retention policy.
func (data *Data) UpdateRetentionPolicy(database, name string, rpu *RetentionPolicyUpdate) error {
// Find database.
di := data.Database(database)
if di == nil {
return ErrDatabaseNotFound
return influxdb.ErrDatabaseNotFound(database)
}

// Find policy.
rpi := di.RetentionPolicy(name)
if rpi == nil {
return ErrRetentionPolicyNotFound
return influxdb.ErrRetentionPolicyNotFound(name)
}

// Ensure new policy doesn't match an existing policy.
Expand Down Expand Up @@ -288,9 +289,9 @@ func (data *Data) SetDefaultRetentionPolicy(database, name string) error {
// Find database and verify policy exists.
di := data.Database(database)
if di == nil {
return ErrDatabaseNotFound
return influxdb.ErrDatabaseNotFound(database)
} else if di.RetentionPolicy(name) == nil {
return ErrRetentionPolicyNotFound
return influxdb.ErrRetentionPolicyNotFound(name)
}

// Set default policy.
Expand All @@ -306,7 +307,7 @@ func (data *Data) ShardGroups(database, policy string) ([]ShardGroupInfo, error)
if err != nil {
return nil, err
} else if rpi == nil {
return nil, ErrRetentionPolicyNotFound
return nil, influxdb.ErrRetentionPolicyNotFound(database)
}
groups := make([]ShardGroupInfo, 0, len(rpi.ShardGroups))
for _, g := range rpi.ShardGroups {
Expand All @@ -326,7 +327,7 @@ func (data *Data) ShardGroupsByTimeRange(database, policy string, tmin, tmax tim
if err != nil {
return nil, err
} else if rpi == nil {
return nil, ErrRetentionPolicyNotFound
return nil, influxdb.ErrRetentionPolicyNotFound(database)
}
groups := make([]ShardGroupInfo, 0, len(rpi.ShardGroups))
for _, g := range rpi.ShardGroups {
Expand All @@ -345,7 +346,7 @@ func (data *Data) ShardGroupByTimestamp(database, policy string, timestamp time.
if err != nil {
return nil, err
} else if rpi == nil {
return nil, ErrRetentionPolicyNotFound
return nil, influxdb.ErrRetentionPolicyNotFound(database)
}

return rpi.ShardGroupByTimestamp(timestamp), nil
Expand All @@ -363,7 +364,7 @@ func (data *Data) CreateShardGroup(database, policy string, timestamp time.Time)
if err != nil {
return err
} else if rpi == nil {
return ErrRetentionPolicyNotFound
return influxdb.ErrRetentionPolicyNotFound(database)
}

// Verify that shard group doesn't already exist for this timestamp.
Expand Down Expand Up @@ -426,7 +427,7 @@ func (data *Data) DeleteShardGroup(database, policy string, id uint64) error {
if err != nil {
return err
} else if rpi == nil {
return ErrRetentionPolicyNotFound
return influxdb.ErrRetentionPolicyNotFound(policy)
}

// Find shard group by ID and set its deletion timestamp.
Expand All @@ -444,7 +445,7 @@ func (data *Data) DeleteShardGroup(database, policy string, id uint64) error {
func (data *Data) CreateContinuousQuery(database, name, query string) error {
di := data.Database(database)
if di == nil {
return ErrDatabaseNotFound
return influxdb.ErrDatabaseNotFound(database)
}

// Ensure the name doesn't already exist.
Expand All @@ -467,7 +468,7 @@ func (data *Data) CreateContinuousQuery(database, name, query string) error {
func (data *Data) DropContinuousQuery(database, name string) error {
di := data.Database(database)
if di == nil {
return ErrDatabaseNotFound
return influxdb.ErrDatabaseNotFound(database)
}

for i := range di.ContinuousQueries {
Expand All @@ -486,7 +487,7 @@ func (data *Data) CreateSubscription(database, rp, name, mode string, destinatio
return err
}
if rpi == nil {
return ErrRetentionPolicyNotFound
return influxdb.ErrRetentionPolicyNotFound(rp)
}

// Ensure the name doesn't already exist.
Expand Down
13 changes: 9 additions & 4 deletions meta/data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/davecgh/go-spew/spew"
"github.com/gogo/protobuf/proto"
"github.com/influxdb/influxdb"
"github.com/influxdb/influxdb/influxql"
"github.com/influxdb/influxdb/meta"
"github.com/influxdb/influxdb/meta/internal"
Expand Down Expand Up @@ -183,7 +184,8 @@ func TestData_CreateRetentionPolicy_ErrReplicationFactorTooLow(t *testing.T) {
// Ensure that creating a retention policy on a non-existent database returns an error.
func TestData_CreateRetentionPolicy_ErrDatabaseNotFound(t *testing.T) {
data := meta.Data{Nodes: []meta.NodeInfo{{ID: 1}}}
if err := data.CreateRetentionPolicy("db0", &meta.RetentionPolicyInfo{Name: "rp0", ReplicaN: 1}); err != meta.ErrDatabaseNotFound {
expErr := influxdb.ErrDatabaseNotFound("db0")
if err := data.CreateRetentionPolicy("db0", &meta.RetentionPolicyInfo{Name: "rp0", ReplicaN: 1}); err.Error() != expErr.Error() {
t.Fatalf("unexpected error: %s", err)
}
}
Expand Down Expand Up @@ -249,7 +251,8 @@ func TestData_DropRetentionPolicy(t *testing.T) {
// Ensure an error is returned when deleting a policy from a non-existent database.
func TestData_DropRetentionPolicy_ErrDatabaseNotFound(t *testing.T) {
var data meta.Data
if err := data.DropRetentionPolicy("db0", "rp0"); err != meta.ErrDatabaseNotFound {
expErr := influxdb.ErrDatabaseNotFound("db0")
if err := data.DropRetentionPolicy("db0", "rp0"); err.Error() != expErr.Error() {
t.Fatal(err)
}
}
Expand All @@ -260,7 +263,8 @@ func TestData_DropRetentionPolicy_ErrRetentionPolicyNotFound(t *testing.T) {
if err := data.CreateDatabase("db0"); err != nil {
t.Fatal(err)
}
if err := data.DropRetentionPolicy("db0", "rp0"); err != meta.ErrRetentionPolicyNotFound {
expErr := influxdb.ErrRetentionPolicyNotFound("rp0")
if err := data.DropRetentionPolicy("db0", "rp0"); err.Error() != expErr.Error() {
t.Fatal(err)
}
}
Expand Down Expand Up @@ -290,7 +294,8 @@ func TestData_RetentionPolicy(t *testing.T) {
// Ensure that retrieving a policy from a non-existent database returns an error.
func TestData_RetentionPolicy_ErrDatabaseNotFound(t *testing.T) {
var data meta.Data
if _, err := data.RetentionPolicy("db0", "rp0"); err != meta.ErrDatabaseNotFound {
expErr := influxdb.ErrDatabaseNotFound("db0")
if _, err := data.RetentionPolicy("db0", "rp0"); err.Error() != expErr.Error() {
t.Fatal(err)
}
}
Expand Down
6 changes: 0 additions & 6 deletions meta/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ var (
// ErrDatabaseExists is returned when creating an already existing database.
ErrDatabaseExists = newError("database already exists")

// ErrDatabaseNotFound is returned when mutating a database that doesn't exist.
ErrDatabaseNotFound = newError("database not found")

// ErrDatabaseNameRequired is returned when creating a database without a name.
ErrDatabaseNameRequired = newError("database name required")
)
Expand All @@ -54,9 +51,6 @@ var (
// on a default retention policy.
ErrRetentionPolicyDefault = newError("retention policy is default")

// ErrRetentionPolicyNotFound is returned when mutating a policy that doesn't exist.
ErrRetentionPolicyNotFound = newError("retention policy not found")

// ErrRetentionPolicyNameRequired is returned when creating a policy without a name.
ErrRetentionPolicyNameRequired = newError("retention policy name required")

Expand Down
3 changes: 2 additions & 1 deletion meta/statement_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strconv"
"time"

"github.com/influxdb/influxdb"
"github.com/influxdb/influxdb/influxql"
"github.com/influxdb/influxdb/models"
)
Expand Down Expand Up @@ -292,7 +293,7 @@ func (e *StatementExecutor) executeShowRetentionPoliciesStatement(q *influxql.Sh
if err != nil {
return &influxql.Result{Err: err}
} else if di == nil {
return &influxql.Result{Err: ErrDatabaseNotFound}
return &influxql.Result{Err: influxdb.ErrDatabaseNotFound(q.Database)}
}

row := &models.Row{Columns: []string{"name", "duration", "replicaN", "default"}}
Expand Down
4 changes: 3 additions & 1 deletion meta/statement_executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

"github.com/davecgh/go-spew/spew"
"github.com/influxdb/influxdb"
"github.com/influxdb/influxdb/influxql"
"github.com/influxdb/influxdb/meta"
"github.com/influxdb/influxdb/models"
Expand Down Expand Up @@ -659,7 +660,8 @@ func TestStatementExecutor_ExecuteStatement_ShowRetentionPolicies_ErrDatabaseNot
return nil, nil
}

if res := e.ExecuteStatement(influxql.MustParseStatement(`SHOW RETENTION POLICIES ON db0`)); res.Err != meta.ErrDatabaseNotFound {
expErr := influxdb.ErrDatabaseNotFound("db0")
if res := e.ExecuteStatement(influxql.MustParseStatement(`SHOW RETENTION POLICIES ON db0`)); res.Err.Error() != expErr.Error() {
t.Fatalf("unexpected error: %s", res.Err)
}
}
Expand Down
5 changes: 3 additions & 2 deletions meta/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"github.com/gogo/protobuf/proto"
"github.com/hashicorp/raft"
"github.com/influxdb/influxdb"
"github.com/influxdb/influxdb/influxql"
"github.com/influxdb/influxdb/meta/internal"
"golang.org/x/crypto/bcrypt"
Expand Down Expand Up @@ -1105,7 +1106,7 @@ func (s *Store) DefaultRetentionPolicy(database string) (rpi *RetentionPolicyInf
err = s.read(func(data *Data) error {
di := data.Database(database)
if di == nil {
return ErrDatabaseNotFound
return influxdb.ErrDatabaseNotFound(database)
}

for i := range di.RetentionPolicies {
Expand All @@ -1124,7 +1125,7 @@ func (s *Store) RetentionPolicies(database string) (a []RetentionPolicyInfo, err
err = s.read(func(data *Data) error {
di := data.Database(database)
if di != nil {
return ErrDatabaseNotFound
return influxdb.ErrDatabaseNotFound(database)
}
a = di.RetentionPolicies
return nil
Expand Down
4 changes: 3 additions & 1 deletion meta/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"testing"
"time"

"github.com/influxdb/influxdb"
"github.com/influxdb/influxdb/meta"
"github.com/influxdb/influxdb/tcp"
"github.com/influxdb/influxdb/toml"
Expand Down Expand Up @@ -240,7 +241,8 @@ func TestStore_DropDatabase_ErrDatabaseNotFound(t *testing.T) {
s := MustOpenStore()
defer s.Close()

if err := s.DropDatabase("no_such_database"); err != meta.ErrDatabaseNotFound {
expErr := influxdb.ErrDatabaseNotFound("no_such_database")
if err := s.DropDatabase("no_such_database"); err.Error() != expErr.Error() {
t.Fatalf("unexpected error: %s", err)
}
}
Expand Down
3 changes: 2 additions & 1 deletion monitor/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"sync"
"time"

"github.com/influxdb/influxdb"
"github.com/influxdb/influxdb/cluster"
"github.com/influxdb/influxdb/meta"
"github.com/influxdb/influxdb/models"
Expand Down Expand Up @@ -329,7 +330,7 @@ func (m *Monitor) createInternalStorage() {
return
}

if err := m.MetaStore.DropRetentionPolicy(m.storeDatabase, "default"); err != nil && err != meta.ErrRetentionPolicyNotFound {
if err := m.MetaStore.DropRetentionPolicy(m.storeDatabase, "default"); err != nil && err != influxdb.ErrRetentionPolicyNotFound("default") {
m.Logger.Printf("failed to delete retention policy 'default', failed to created internal storage: %s", err.Error())
return
}
Expand Down

0 comments on commit 657877d

Please sign in to comment.