Skip to content

Commit

Permalink
refactor: restructure modules (#25)
Browse files Browse the repository at this point in the history
* refactor!: major code restructuring to decouple discovery module

* refactor: remove unused configs

* chore: update go version to 1.16

* refactor: use RecordType.String() instead of string casting

* refactor: simplify variable names

* feat(type): add type as a resource again
  • Loading branch information
StewartJingga authored Dec 7, 2021
1 parent c12e9d4 commit b94e337
Show file tree
Hide file tree
Showing 61 changed files with 2,536 additions and 2,261 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
- uses: actions/checkout@v2
- uses: actions/setup-go@v2
with:
go-version: 1.13
go-version: 1.16
- name: Install dependencies
run: sudo apt-get install build-essential
- name: Install packages
Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM golang:1.13-stretch as base
FROM golang:1.16-stretch as base
WORKDIR /build/
COPY . .
RUN ["make"]
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Explore the following resources to get started with Columbus:

## Requirements

Columbus is written in golang, and requires go version >= 1.13. Please make sure that the go tool chain is available on your machine. See golang’s [documentation](https://golang.org/) for installation instructions.
Columbus is written in golang, and requires go version >= 1.16. Please make sure that the go tool chain is available on your machine. See golang’s [documentation](https://golang.org/) for installation instructions.

Alternatively, you can use docker to build columbus as a docker image. More on this in the next section.

Expand Down
19 changes: 15 additions & 4 deletions api/handlers/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,29 @@ package handlers

import (
"encoding/json"
"fmt"
"log"
"net/http"
)

func writeJSON(w http.ResponseWriter, status int, v interface{}) error {
func writeJSON(w http.ResponseWriter, status int, v interface{}) {
w.Header().Set("content-type", "application/json")
w.WriteHeader(status)
return json.NewEncoder(w).Encode(v)
err := json.NewEncoder(w).Encode(v)
if err != nil {
w.Header().Set("content-type", "application/json")
w.WriteHeader(status)
code, err := w.Write([]byte("error encoding response to json"))
if err != nil {
log.Print(fmt.Sprintf("error writing response with code: %d", code))
}
}
}

func writeJSONError(w http.ResponseWriter, status int, msg string) error {
func writeJSONError(w http.ResponseWriter, status int, msg string) {
response := &ErrorResponse{
Reason: msg,
}
return writeJSON(w, status, response)

writeJSON(w, status, response)
}
7 changes: 4 additions & 3 deletions api/handlers/lineage_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (

"github.com/gorilla/mux"
"github.com/odpf/columbus/lineage"
"github.com/odpf/columbus/models"
"github.com/odpf/columbus/record"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -51,12 +51,13 @@ func (handler *LineageHandler) ListLineage(w http.ResponseWriter, r *http.Reques
Errorf("error querying graph: %v\ncfg: %v", err, opts)

status := http.StatusBadRequest
if _, ok := err.(models.ErrNoSuchType); ok {
if _, ok := err.(record.ErrNoSuchType); ok {
status = http.StatusNotFound
}
writeJSONError(w, status, err.Error())
return
}

writeJSON(w, http.StatusOK, res)
}

Expand All @@ -80,7 +81,7 @@ func (handler *LineageHandler) GetLineage(w http.ResponseWriter, r *http.Request
Errorf("error querying graph: %v\ncfg: %v", err, opts)

status := http.StatusBadRequest
if _, ok := err.(models.ErrNoSuchType); ok {
if _, ok := err.(record.ErrNoSuchType); ok {
status = http.StatusNotFound
}
writeJSONError(w, status, err.Error())
Expand Down
58 changes: 25 additions & 33 deletions api/handlers/lineage_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"github.com/odpf/columbus/lib/mock"
"github.com/odpf/columbus/lib/set"
"github.com/odpf/columbus/lineage"
"github.com/odpf/columbus/models"
"github.com/odpf/columbus/record"
)

func TestLineageHandler(t *testing.T) {
Expand All @@ -22,15 +22,13 @@ func TestLineageHandler(t *testing.T) {
graph := new(mock.Graph)
graph.On(
"Query",
lineage.QueryCfg{TypeWhitelist: []string{"bqtable"}}).Return(lineage.AdjacencyMap{}, models.ErrNoSuchType{"bqtable"})
lineage.QueryCfg{TypeWhitelist: []string{"bqtable"}}).Return(lineage.AdjacencyMap{}, record.ErrNoSuchType{TypeName: "bqtable"})
lp := new(mock.LineageProvider)
lp.On("Graph").Return(graph, nil)

handler := handlers.NewLineageHandler(new(mock.Logger), lp)

rr := httptest.NewRequest("GET", "/?filter.type=bqtable", nil)
rw := httptest.NewRecorder()

handler.ListLineage(rw, rr)

if rw.Code != http.StatusNotFound {
Expand All @@ -44,42 +42,37 @@ func TestLineageHandler(t *testing.T) {
t.Errorf("error decoding handler response: %v", err)
return
}

expectedReason := `no such type: "bqtable"`
if response.Reason != expectedReason {
t.Errorf("expected handler to report reason %q, was %q instead", expectedReason, response.Reason)
}
})
t.Run("should return graph filtered by type", func(t *testing.T) {
var filteredGraph = lineage.AdjacencyMap{
"topic/a": lineage.AdjacencyEntry{
Type: "topic",
URN: "a",
Upstreams: set.NewStringSet(),
Downstreams: set.NewStringSet("dagger/ab"),
Downstreams: set.NewStringSet("table/ab"),
},
"topic/b": lineage.AdjacencyEntry{
Type: "topic",
URN: "a",
Upstreams: set.NewStringSet("dagger/ab"),
Upstreams: set.NewStringSet("table/ab"),
Downstreams: set.NewStringSet(),
},
"dagger/ab": lineage.AdjacencyEntry{
Type: "dagger",
"table/ab": lineage.AdjacencyEntry{
Type: "table",
URN: "ab",
Upstreams: set.NewStringSet("topic/a"),
Downstreams: set.NewStringSet("topic/b"),
},
}
graph := new(mock.Graph)
graph.On("Query", lineage.QueryCfg{TypeWhitelist: []string{"topic", "dagger"}}).Return(filteredGraph, nil)
graph.On("Query", lineage.QueryCfg{TypeWhitelist: []string{"topic", "table"}}).Return(filteredGraph, nil)

lp := new(mock.LineageProvider)
lp.On("Graph").Return(graph, nil)

handler := handlers.NewLineageHandler(new(mock.Logger), lp)

rr := httptest.NewRequest("GET", "/?filter.type=topic&filter.type=dagger", nil)
rr := httptest.NewRequest("GET", "/?filter.type=topic&filter.type=table", nil)
rw := httptest.NewRecorder()

handler.ListLineage(rw, rr)
Expand Down Expand Up @@ -119,19 +112,18 @@ func TestLineageHandler(t *testing.T) {
}
})
t.Run("should return the entire graph if no options are specified", func(t *testing.T) {

// using the same graph
var fullGraph = lineage.AdjacencyMap{
"bqtable/raw": lineage.AdjacencyEntry{
Type: "bqtable",
"table/raw": lineage.AdjacencyEntry{
Type: "table",
URN: "raw",
Upstreams: set.NewStringSet(),
Downstreams: set.NewStringSet("s3/std"),
Downstreams: set.NewStringSet("topic/std"),
},
"s3/std": lineage.AdjacencyEntry{
Type: "s3",
"topic/std": lineage.AdjacencyEntry{
Type: "topic",
URN: "std",
Upstreams: set.NewStringSet("bqtable/raw"),
Upstreams: set.NewStringSet("table/raw"),
Downstreams: set.NewStringSet(),
},
}
Expand Down Expand Up @@ -168,38 +160,38 @@ func TestLineageHandler(t *testing.T) {
t.Run("GetLineage", func(t *testing.T) {
t.Run("should return a graph containing the requested resource, along with it's related resources", func(t *testing.T) {
var subGraph = lineage.AdjacencyMap{
"bqtable/raw": lineage.AdjacencyEntry{
Type: "bqtable",
"table/raw": lineage.AdjacencyEntry{
Type: "table",
URN: "raw",
Upstreams: set.NewStringSet(),
Downstreams: set.NewStringSet("bqtable/std"),
Downstreams: set.NewStringSet("table/std"),
},
"bqtable/std": lineage.AdjacencyEntry{
Type: "bqtable",
"table/std": lineage.AdjacencyEntry{
Type: "table",
URN: "std",
Upstreams: set.NewStringSet("bqtable/raw", "bqtable/to-be-removed"),
Upstreams: set.NewStringSet("table/raw", "table/to-be-removed"),
Downstreams: set.NewStringSet(),
},
"bqtable/presentation": lineage.AdjacencyEntry{
Type: "bqtable",
"table/presentation": lineage.AdjacencyEntry{
Type: "table",
URN: "presentation",
Upstreams: set.NewStringSet(),
Downstreams: set.NewStringSet(),
},
}

graph := new(mock.Graph)
graph.On("Query", lineage.QueryCfg{Root: "bqtable/raw"}).Return(subGraph, nil)
graph.On("Query", lineage.QueryCfg{Root: "table/raw"}).Return(subGraph, nil)

lp := new(mock.LineageProvider)
lp.On("Graph").Return(graph, nil)

handler := handlers.NewLineageHandler(new(mock.Logger), lp)

rr := httptest.NewRequest("GET", "/v1/lineage/bqtable/raw", nil)
rr := httptest.NewRequest("GET", "/v1/lineage/table/raw", nil)
rw := httptest.NewRecorder()
rr = mux.SetURLVars(rr, map[string]string{
"type": "bqtable",
"type": "table",
"id": "raw",
})

Expand Down
Loading

0 comments on commit b94e337

Please sign in to comment.