Skip to content

Commit

Permalink
Merge pull request #1990 from influxdata/convert_scraper_errors
Browse files Browse the repository at this point in the history
fix(http): convert scraper target error
  • Loading branch information
kelwang authored Dec 17, 2018
2 parents 49d0983 + 4cabda4 commit 014dda0
Show file tree
Hide file tree
Showing 8 changed files with 213 additions and 113 deletions.
91 changes: 71 additions & 20 deletions bolt/scraper.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ package bolt
import (
"context"
"encoding/json"
"errors"
"fmt"

bolt "github.com/coreos/bbolt"
"github.com/influxdata/platform"
Expand Down Expand Up @@ -37,71 +35,124 @@ func (c *Client) ListTargets(ctx context.Context) (list []platform.ScraperTarget
}
return err
})
if err != nil {
return nil, &platform.Error{
Op: getOp(platform.OpListTargets),
Err: err,
}
}
return list, err
}

// AddTarget add a new scraper target into storage.
func (c *Client) AddTarget(ctx context.Context, target *platform.ScraperTarget) (err error) {
return c.db.Update(func(tx *bolt.Tx) error {
err = c.db.Update(func(tx *bolt.Tx) error {
target.ID = c.IDGenerator.ID()
return c.putTarget(ctx, tx, target)
})
if err != nil {
return &platform.Error{
Err: err,
Op: OpPrefix + platform.OpAddTarget,
}
}
return nil
}

// RemoveTarget removes a scraper target from the bucket.
func (c *Client) RemoveTarget(ctx context.Context, id platform.ID) error {
return c.db.Update(func(tx *bolt.Tx) error {
_, err := c.findTargetByID(ctx, tx, id)
if err != nil {
return err
err := c.db.Update(func(tx *bolt.Tx) error {
_, pe := c.findTargetByID(ctx, tx, id)
if pe != nil {
return pe
}
encID, err := id.Encode()
if err != nil {
return err
return &platform.Error{
Code: platform.EInvalid,
Err: err,
}
}
return tx.Bucket(scraperBucket).Delete(encID)
})
if err != nil {
return &platform.Error{
Err: err,
Op: OpPrefix + platform.OpRemoveTarget,
}
}
return nil
}

// UpdateTarget updates a scraper target.
func (c *Client) UpdateTarget(ctx context.Context, update *platform.ScraperTarget) (target *platform.ScraperTarget, err error) {
op := getOp(platform.OpUpdateTarget)
var pe *platform.Error
if !update.ID.Valid() {
return nil, errors.New("update scraper: id is invalid")
return nil, &platform.Error{
Code: platform.EInvalid,
Op: op,
Msg: "id is invalid",
}
}
err = c.db.Update(func(tx *bolt.Tx) error {
target, err = c.findTargetByID(ctx, tx, update.ID)
if err != nil {
return err
target, pe = c.findTargetByID(ctx, tx, update.ID)
if pe != nil {
return pe
}
target = update
return c.putTarget(ctx, tx, target)
})

return target, err
if err != nil {
return nil, &platform.Error{
Op: op,
Err: err,
}
}

return target, nil
}

// GetTargetByID retrieves a scraper target by id.
func (c *Client) GetTargetByID(ctx context.Context, id platform.ID) (target *platform.ScraperTarget, err error) {
var pe *platform.Error
err = c.db.View(func(tx *bolt.Tx) error {
target, err = c.findTargetByID(ctx, tx, id)
return err
target, pe = c.findTargetByID(ctx, tx, id)
if pe != nil {
return &platform.Error{
Op: getOp(platform.OpGetTargetByID),
Err: pe,
}
}
return nil
})
return target, err
if err != nil {
return nil, err
}
return target, nil
}

func (c *Client) findTargetByID(ctx context.Context, tx *bolt.Tx, id platform.ID) (target *platform.ScraperTarget, err error) {
func (c *Client) findTargetByID(ctx context.Context, tx *bolt.Tx, id platform.ID) (target *platform.ScraperTarget, pe *platform.Error) {
target = new(platform.ScraperTarget)
encID, err := id.Encode()
if err != nil {
return nil, err
return nil, &platform.Error{
Err: err,
}
}
v := tx.Bucket(scraperBucket).Get(encID)
if len(v) == 0 {
return nil, fmt.Errorf("scraper target is not found")
return nil, &platform.Error{
Code: platform.ENotFound,
Msg: "scraper target is not found",
}
}

if err := json.Unmarshal(v, target); err != nil {
return nil, err
return nil, &platform.Error{
Err: err,
}
}
return target, nil
}
Expand Down
5 changes: 3 additions & 2 deletions bolt/scraper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ import (
"testing"

"github.com/influxdata/platform"
"github.com/influxdata/platform/bolt"
platformtesting "github.com/influxdata/platform/testing"
)

func initScraperTargetStoreService(f platformtesting.TargetFields, t *testing.T) (platform.ScraperTargetStoreService, func()) {
func initScraperTargetStoreService(f platformtesting.TargetFields, t *testing.T) (platform.ScraperTargetStoreService, string, func()) {
c, closeFn, err := NewTestClient()
if err != nil {
t.Fatalf("failed to create new bolt client: %v", err)
Expand All @@ -20,7 +21,7 @@ func initScraperTargetStoreService(f platformtesting.TargetFields, t *testing.T)
t.Fatalf("failed to populate users")
}
}
return c, func() {
return c, bolt.OpPrefix, func() {
defer closeFn()
for _, target := range f.Targets {
if err := c.RemoveTarget(ctx, target.ID); err != nil {
Expand Down
19 changes: 12 additions & 7 deletions http/scraper_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"net/http"
"path"

Expand Down Expand Up @@ -174,6 +173,8 @@ type ScraperService struct {
Addr string
Token string
InsecureSkipVerify bool
// OpPrefix is for update invalid ops
OpPrefix string
}

// ListTargets returns a list of all scraper targets.
Expand All @@ -198,7 +199,7 @@ func (s *ScraperService) ListTargets(ctx context.Context) ([]platform.ScraperTar
if err != nil {
return nil, err
}
if err := CheckError(resp); err != nil {
if err := CheckError(resp, true); err != nil {
return nil, err
}

Expand All @@ -219,7 +220,11 @@ func (s *ScraperService) ListTargets(ctx context.Context) ([]platform.ScraperTar
// Returns the new target state after update.
func (s *ScraperService) UpdateTarget(ctx context.Context, update *platform.ScraperTarget) (*platform.ScraperTarget, error) {
if !update.ID.Valid() {
return nil, errors.New("update scraper: id is invalid")
return nil, &platform.Error{
Code: platform.EInvalid,
Op: s.OpPrefix + platform.OpUpdateTarget,
Msg: "id is invalid",
}
}
url, err := newURL(s.Addr, targetIDPath(update.ID))
if err != nil {
Expand All @@ -244,7 +249,7 @@ func (s *ScraperService) UpdateTarget(ctx context.Context, update *platform.Scra
return nil, err
}

if err := CheckError(resp); err != nil {
if err := CheckError(resp, true); err != nil {
return nil, err
}
var targetResp targetResponse
Expand Down Expand Up @@ -284,7 +289,7 @@ func (s *ScraperService) AddTarget(ctx context.Context, target *platform.Scraper
}

// TODO(jsternberg): Should this check for a 201 explicitly?
if err := CheckError(resp); err != nil {
if err := CheckError(resp, true); err != nil {
return err
}

Expand Down Expand Up @@ -315,7 +320,7 @@ func (s *ScraperService) RemoveTarget(ctx context.Context, id platform.ID) error
return err
}

return CheckErrorStatus(http.StatusNoContent, resp)
return CheckErrorStatus(http.StatusNoContent, resp, true)
}

// GetTargetByID returns a single target by ID.
Expand All @@ -337,7 +342,7 @@ func (s *ScraperService) GetTargetByID(ctx context.Context, id platform.ID) (*pl
return nil, err
}

if err := CheckError(resp); err != nil {
if err := CheckError(resp, true); err != nil {
return nil, err
}

Expand Down
7 changes: 4 additions & 3 deletions http/scraper_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
platformtesting "github.com/influxdata/platform/testing"
)

func initScraperService(f platformtesting.TargetFields, t *testing.T) (platform.ScraperTargetStoreService, func()) {
func initScraperService(f platformtesting.TargetFields, t *testing.T) (platform.ScraperTargetStoreService, string, func()) {
t.Helper()
svc := inmem.NewService()
svc.IDGenerator = f.IDGenerator
Expand All @@ -26,11 +26,12 @@ func initScraperService(f platformtesting.TargetFields, t *testing.T) (platform.
handler.ScraperStorageService = svc
server := httptest.NewServer(handler)
client := ScraperService{
Addr: server.URL,
Addr: server.URL,
OpPrefix: inmem.OpPrefix,
}
done := server.Close

return &client, done
return &client, inmem.OpPrefix, done
}

func TestScraperService(t *testing.T) {
Expand Down
72 changes: 55 additions & 17 deletions inmem/scraper.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,32 @@ package inmem

import (
"context"
"errors"
"fmt"

"github.com/influxdata/platform"
)

var (
errScraperTargetNotFound = fmt.Errorf("scraper target is not found")
const (
errScraperTargetNotFound = "scraper target is not found"
)

var _ platform.ScraperTargetStoreService = (*Service)(nil)

func (s *Service) loadScraperTarget(id platform.ID) (*platform.ScraperTarget, error) {
func (s *Service) loadScraperTarget(id platform.ID) (*platform.ScraperTarget, *platform.Error) {
i, ok := s.scraperTargetKV.Load(id.String())
if !ok {
return nil, errScraperTargetNotFound
return nil, &platform.Error{
Code: platform.ENotFound,
Msg: errScraperTargetNotFound,
}
}

b, ok := i.(platform.ScraperTarget)
if !ok {
return nil, fmt.Errorf("type %T is not a scraper target", i)
return nil, &platform.Error{
Code: platform.EInvalid,
Msg: fmt.Sprintf("type %T is not a scraper target", i),
}
}
return &b, nil
}
Expand All @@ -33,7 +38,10 @@ func (s *Service) ListTargets(ctx context.Context) (list []platform.ScraperTarge
s.scraperTargetKV.Range(func(_, v interface{}) bool {
b, ok := v.(platform.ScraperTarget)
if !ok {
err = fmt.Errorf("type %T is not a scraper target", v)
err = &platform.Error{
Code: platform.EInvalid,
Msg: fmt.Sprintf("type %T is not a scraper target", v),
}
return false
}
list = append(list, b)
Expand All @@ -45,34 +53,64 @@ func (s *Service) ListTargets(ctx context.Context) (list []platform.ScraperTarge
// AddTarget add a new scraper target into storage.
func (s *Service) AddTarget(ctx context.Context, target *platform.ScraperTarget) (err error) {
target.ID = s.IDGenerator.ID()
return s.PutTarget(ctx, target)
if err := s.PutTarget(ctx, target); err != nil {
return &platform.Error{
Op: OpPrefix + platform.OpAddTarget,
Err: err,
}
}
return nil
}

// RemoveTarget removes a scraper target from the bucket.
func (s *Service) RemoveTarget(ctx context.Context, id platform.ID) error {
if _, err := s.loadScraperTarget(id); err != nil {
return err
if _, pe := s.loadScraperTarget(id); pe != nil {
return &platform.Error{
Err: pe,
Op: OpPrefix + platform.OpRemoveTarget,
}
}
s.scraperTargetKV.Delete(id.String())
return nil
}

// UpdateTarget updates a scraper target.
func (s *Service) UpdateTarget(ctx context.Context, update *platform.ScraperTarget) (target *platform.ScraperTarget, err error) {
op := OpPrefix + platform.OpUpdateTarget
if !update.ID.Valid() {
return nil, errors.New("update scraper: id is invalid")
return nil, &platform.Error{
Code: platform.EInvalid,
Op: op,
Msg: "id is invalid",
}
}
_, err = s.loadScraperTarget(update.ID)
if err != nil {
return nil, err
_, pe := s.loadScraperTarget(update.ID)
if pe != nil {
return nil, &platform.Error{
Op: op,
Err: pe,
}
}
if err = s.PutTarget(ctx, update); err != nil {
return nil, &platform.Error{
Op: op,
Err: pe,
}
}
err = s.PutTarget(ctx, update)
return update, err

return update, nil
}

// GetTargetByID retrieves a scraper target by id.
func (s *Service) GetTargetByID(ctx context.Context, id platform.ID) (target *platform.ScraperTarget, err error) {
return s.loadScraperTarget(id)
var pe *platform.Error
if target, pe = s.loadScraperTarget(id); pe != nil {
return nil, &platform.Error{
Op: OpPrefix + platform.OpGetTargetByID,
Err: pe,
}
}
return target, nil
}

// PutTarget will put a scraper target without setting an ID.
Expand Down
Loading

0 comments on commit 014dda0

Please sign in to comment.