Skip to content

Commit

Permalink
fix(lint): fix linting error (#69)
Browse files Browse the repository at this point in the history
* chore: add lint in CI

* chore: fix lint error

* fix: import errors

* refactor: update error handling

* refactor: error package
  • Loading branch information
scortier authored Jan 25, 2022
1 parent c19b5a6 commit 342ef0b
Show file tree
Hide file tree
Showing 25 changed files with 115 additions and 176 deletions.
19 changes: 19 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: lint

on: [push, pull_request]

jobs:
golangci:
runs-on: ubuntu-latest
steps:
- uses: actions/setup-go@v2
with:
go-version: '^1.16'
- uses: actions/checkout@v2
with:
fetch-depth: 0
- name: golangci-lint
uses: golangci/golangci-lint-action@v2
with:
skip-go-installation: true
version: v1.42.1
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,7 @@ test-coverage: test

dist:
@bash ./scripts/build.sh

lint:
golangci-lint run

4 changes: 2 additions & 2 deletions api/handlers/record_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ func TestRecordHandler(t *testing.T) {
var response discovery.RecordList
err := json.NewDecoder(resp.Body).Decode(&response)
if err != nil {
return fmt.Errorf("error parsing response payload: %v", err)
return fmt.Errorf("error parsing response payload: %w", err)
}

expected := discovery.RecordList{
Expand Down Expand Up @@ -436,7 +436,7 @@ func TestRecordHandler(t *testing.T) {
var response discovery.RecordList
err := json.NewDecoder(resp.Body).Decode(&response)
if err != nil {
return fmt.Errorf("error parsing response payload: %v", err)
return fmt.Errorf("error parsing response payload: %w", err)
}

expected := discovery.RecordList{
Expand Down
4 changes: 1 addition & 3 deletions api/handlers/search_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,7 @@ func (handler *SearchHandler) buildSearchCfg(params url.Values) (cfg discovery.S

func parseTypeWhiteList(values url.Values) (types []string, err error) {
for _, commaSeparatedTypes := range values[whiteListQueryParamKey] {
for _, ts := range strings.Split(commaSeparatedTypes, ",") {
types = append(types, ts)
}
types = append(types, strings.Split(commaSeparatedTypes, ",")...)
}
return
}
6 changes: 3 additions & 3 deletions api/handlers/search_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func TestSearchHandlerSearch(t *testing.T) {
var response []handlers.SearchResponse
err := json.NewDecoder(body).Decode(&response)
if err != nil {
return fmt.Errorf("error reading response body: %v", err)
return fmt.Errorf("error reading response body: %w", err)
}
expectResponse := []handlers.SearchResponse{
{
Expand Down Expand Up @@ -176,7 +176,7 @@ func TestSearchHandlerSearch(t *testing.T) {
var payload []interface{}
err := json.NewDecoder(body).Decode(&payload)
if err != nil {
return fmt.Errorf("error reading response body: %v", err)
return fmt.Errorf("error reading response body: %w", err)
}

expectedSize := 10
Expand Down Expand Up @@ -299,7 +299,7 @@ func TestSearchHandlerSuggest(t *testing.T) {
var response handlers.SuggestResponse
err := json.NewDecoder(body).Decode(&response)
if err != nil {
return fmt.Errorf("error reading response body: %v", err)
return fmt.Errorf("error reading response body: %w", err)
}
expectResponse := handlers.SuggestResponse{
Suggestions: []string{
Expand Down
60 changes: 2 additions & 58 deletions api/handlers/type_handler.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
package handlers

import (
"fmt"
"github.com/odpf/columbus/record"
"github.com/odpf/salt/log"
"net/http"
"strings"

"github.com/odpf/columbus/record"
)

// TypeHandler exposes a REST interface to types
Expand Down Expand Up @@ -38,7 +35,7 @@ func (h *TypeHandler) Get(w http.ResponseWriter, r *http.Request) {

results := []TypeWithCount{}
for _, typName := range record.AllSupportedTypes {
count, _ := typesNameMap[typName]
count := typesNameMap[typName]
results = append(results, TypeWithCount{
Name: typName.String(),
Count: count,
Expand All @@ -47,56 +44,3 @@ func (h *TypeHandler) Get(w http.ResponseWriter, r *http.Request) {

writeJSON(w, http.StatusOK, results)
}

func (h *TypeHandler) parseSelectQuery(raw string) (fields []string) {
tokens := strings.Split(raw, ",")
for _, token := range tokens {
token = strings.TrimSpace(token)
if token == "" {
continue
}
fields = append(fields, token)
}
return
}

func (h *TypeHandler) selectRecordFields(fields []string, records []record.Record) (processedRecords []record.Record) {
for _, record := range records {
newData := map[string]interface{}{}
for _, field := range fields {
v, ok := record.Data[field]
if !ok {
continue
}
newData[field] = v
}
record.Data = newData
processedRecords = append(processedRecords, record)
}
return
}

func (h *TypeHandler) validateRecord(record record.Record) error {
if record.Urn == "" {
return fmt.Errorf("urn is required")
}
if record.Name == "" {
return fmt.Errorf("name is required")
}
if record.Data == nil {
return fmt.Errorf("data is required")
}
if record.Service == "" {
return fmt.Errorf("service is required")
}

return nil
}

func (h *TypeHandler) responseStatusForError(err error) (int, string) {
switch err.(type) {
case record.ErrNoSuchType, record.ErrNoSuchRecord:
return http.StatusNotFound, err.Error()
}
return http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)
}
6 changes: 2 additions & 4 deletions cmd/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"github.com/odpf/columbus/record"
esStore "github.com/odpf/columbus/store/elasticsearch"
"github.com/odpf/columbus/store/postgres"
"github.com/pkg/errors"
)

const (
Expand Down Expand Up @@ -58,8 +57,7 @@ func migratePostgres(logger log.Logger) (err error) {

err = pgClient.Migrate(pgConfig)
if err != nil {
return errors.Wrap(err, "problem with migration")

return fmt.Errorf("problem with migration %w", err)
}

return nil
Expand All @@ -74,7 +72,7 @@ func migrateElasticsearch(logger log.Logger) (err error) {
defer cancel()
err = esStore.Migrate(ctx, esClient, supportedTypeName)
if err != nil {
err = errors.Wrapf(err, "error creating/replacing type: %q", supportedTypeName)
err = fmt.Errorf("error creating/replacing type %q: %w", supportedTypeName, err)
return
}
logger.Info("created/updated type\n", "type", supportedTypeName)
Expand Down
8 changes: 4 additions & 4 deletions discovery/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ package discovery

import (
"context"
"fmt"

"github.com/odpf/columbus/record"
"github.com/pkg/errors"
)

type Service struct {
Expand All @@ -22,12 +22,12 @@ func NewService(factory RecordRepositoryFactory, recordSearcher RecordSearcher)
func (s *Service) Upsert(ctx context.Context, typeName string, records []record.Record) (err error) {
repo, err := s.factory.For(typeName)
if err != nil {
return errors.Wrapf(err, "error building repo for type \"%s\"", typeName)
return fmt.Errorf("error building repo for type \"%s\": %w", typeName, err)
}

err = repo.CreateOrReplaceMany(ctx, records)
if err != nil {
return errors.Wrap(err, "error upserting records")
return fmt.Errorf("error upserting records: %w", err)
}

return nil
Expand All @@ -36,7 +36,7 @@ func (s *Service) Upsert(ctx context.Context, typeName string, records []record.
func (s *Service) DeleteRecord(ctx context.Context, typeName string, recordURN string) error {
repo, err := s.factory.For(typeName)
if err != nil {
return errors.Wrapf(err, "error building repo for type \"%s\"", typeName)
return fmt.Errorf("error building repo for type \"%s\": %w", typeName, err)
}

err = repo.Delete(ctx, recordURN)
Expand Down
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ require (
github.com/odpf/salt v0.0.0-20220106155451-62e8c849ae81
github.com/olivere/elastic/v7 v7.0.31
github.com/ory/dockertest/v3 v3.8.1
github.com/pkg/errors v0.9.1
github.com/sirupsen/logrus v1.8.1
github.com/spf13/cobra v1.3.0
github.com/spf13/viper v1.10.1
Expand Down
3 changes: 0 additions & 3 deletions lineage/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ func (builder defaultBuilder) addRecord(graph AdjacencyMap, typeName string, rec
Downstreams: builder.buildAdjacents(record, dataflowDirDownstream),
Upstreams: builder.buildAdjacents(record, dataflowDirUpstream),
}

graph[entry.ID()] = entry
}

Expand Down Expand Up @@ -121,8 +120,6 @@ func (builder defaultBuilder) addBackRef(graph AdjacencyMap, refID string, backR
if _, exists := adjacents[refID]; !exists {
adjacents.Add(refID)
}

return
}

func (builder defaultBuilder) buildAdjacents(r record.Record, dir dataflowDir) (adjacents set.StringSet) {
Expand Down
13 changes: 10 additions & 3 deletions lineage/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ func TestCachedGraph(t *testing.T) {
Description: "simple cache test",
Cfg: lineage.QueryCfg{},
Setup: func(tc testCase, g *lineage.CachedGraph) {
g.Query(tc.Cfg) // cache the request
// cache the request
if _, err := g.Query(tc.Cfg); err != nil {
t.Fatal(err)
}
},
Graph: graphFromTestCase,
ExpectResult: lineage.AdjacencyMap{},
Expand Down Expand Up @@ -83,9 +86,13 @@ func TestCachedGraph(t *testing.T) {
)
enc.SetIndent("", " ")
fmt.Fprint(msg, "expected: ")
enc.Encode(tc.ExpectResult)
if err := enc.Encode(tc.ExpectResult); err != nil {
t.Fatal(err)
}
fmt.Fprint(msg, "got: ")
enc.Encode(result)
if err := enc.Encode(result); err != nil {
t.Fatal(err)
}
t.Error(msg.String())
return
}
Expand Down
31 changes: 1 addition & 30 deletions lineage/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (

"github.com/odpf/columbus/lib/set"
"github.com/odpf/columbus/record"
"github.com/pkg/errors"
)

type QueryCfg struct {
Expand All @@ -32,7 +31,6 @@ func (graph *InMemoryGraph) init() {
}
values.Add(id)
}
return
}

func (graph InMemoryGraph) Query(cfg QueryCfg) (AdjacencyMap, error) {
Expand All @@ -59,7 +57,7 @@ func (graph InMemoryGraph) Query(cfg QueryCfg) (AdjacencyMap, error) {
var err error
supergraph, err = graph.buildSubgraphFromRoot(supergraph, cfg.Root)
if err != nil {
return supergraph, errors.Wrap(err, "error building subgraph")
return supergraph, fmt.Errorf("error building subgraph: %w", err)
}
}

Expand All @@ -86,7 +84,6 @@ func (graph InMemoryGraph) collapse(subgraph AdjacencyMap, typeWhitelist set.Str
entry.Downstreams = graph.collapseInDir(entry, dataflowDirDownstream, typeWhitelist)
subgraph[entry.ID()] = entry
}
return
}

func (graph InMemoryGraph) collapseInDir(root AdjacencyEntry, dir dataflowDir, types set.StringSet) set.StringSet {
Expand Down Expand Up @@ -129,32 +126,6 @@ func (graph InMemoryGraph) addAdjacentsInDir(subgraph AdjacencyMap, superGraph A
queue = append(queue, adjacentEl)
}
}
return
}

// remove refs for each entry that don't exist in the built graph
// during handler.addAdjacentsInDir we follow the declarations to add
// upstreams/downstreams to the graph. Once that is done, we've constructed the lineage
// graph for the requested resource. During this step, we remove outgoing refs to any
// resource that doesn't belong in the graph
func (graph InMemoryGraph) pruneRefs(subgraph AdjacencyMap) AdjacencyMap {
pruned := make(AdjacencyMap)
for _, entry := range subgraph {
entry.Upstreams = graph.filterRefs(entry.Upstreams, subgraph)
entry.Downstreams = graph.filterRefs(entry.Downstreams, subgraph)
pruned[entry.ID()] = entry
}
return pruned
}

func (graph InMemoryGraph) filterRefs(refs set.StringSet, subgraph AdjacencyMap) set.StringSet {
rv := set.NewStringSet()
for ref := range refs {
if _, exists := subgraph[ref]; exists {
rv.Add(ref)
}
}
return rv
}

func NewInMemoryGraph(data AdjacencyMap) InMemoryGraph {
Expand Down
8 changes: 6 additions & 2 deletions lineage/graph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,13 @@ func TestInMemoryGraph(t *testing.T) {
)
enc.SetIndent("", " ")
fmt.Fprint(msg, "expected: ")
enc.Encode(tc.ExpectGraph)
if err := enc.Encode(tc.ExpectGraph); err != nil {
t.Fatal(err)
}
fmt.Fprint(msg, "got: ")
enc.Encode(result)
if err := enc.Encode(result); err != nil {
t.Fatal(err)
}
t.Error(msg.String())
return
}
Expand Down
Loading

0 comments on commit 342ef0b

Please sign in to comment.