Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring project structure #9

Merged
merged 1 commit into from
Nov 23, 2023
Merged

Refactoring project structure #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,25 @@
## Makefile for Tarmac Example Project

build:
## Create build directory
mkdir -p functions/build
## Build Init Function
docker run --rm -v `pwd`:/build -w /build/functions/build/init tinygo/tinygo:0.25.0 tinygo build -o /build/functions/build/init.wasm -target wasi /build/functions/src/init/main.go
mkdir -p functions/build/data
docker run --rm -v `pwd`:/build -w /build/functions/build/data/init tinygo/tinygo:0.25.0 tinygo build -o /build/functions/build/data/init.wasm -target wasi /build/functions/src/data/init/main.go
## Build CSV Fetch Function
docker run --rm -v `pwd`:/build -w /build/functions/build/data/fetch tinygo/tinygo:0.25.0 tinygo build -o /build/functions/build/fetch.wasm -target wasi /build/functions/src/data/fetch/main.go
mkdir -p functions/build/data
docker run --rm -v `pwd`:/build -w /build/functions/build/data/fetch tinygo/tinygo:0.25.0 tinygo build -o /build/functions/build/data/fetch.wasm -target wasi /build/functions/src/data/fetch/main.go
## Build CSV Load Function
docker run --rm -v `pwd`:/build -w /build/functions/build/data/load tinygo/tinygo:0.25.0 tinygo build -o /build/functions/build/load.wasm -target wasi /build/functions/src/data/load/main.go
mkdir -p functions/build/data
docker run --rm -v `pwd`:/build -w /build/functions/build/data/load tinygo/tinygo:0.25.0 tinygo build -o /build/functions/build/data/load.wasm -target wasi /build/functions/src/data/load/main.go
## Build HTTP Request Handler Function
docker run --rm -v `pwd`:/build -w /build/functions/build/handler tinygo/tinygo:0.25.0 tinygo build -o /build/functions/build/handler.wasm -target wasi /build/functions/src/handler/main.go
mkdir -p functions/build/handlers
docker run --rm -v `pwd`:/build -w /build/functions/build/handlers/lookup/ tinygo/tinygo:0.25.0 tinygo build -o /build/functions/build/handlers/lookup.wasm -target wasi /build/functions/src/handlers/lookup/main.go

.PHONY: tests
tests:
## Run tests
mkdir -p coverage
go test -v -race -covermode=atomic -coverprofile=coverage/coverage.out ./...
go tool cover -html=coverage/coverage.out -o coverage/coverage.html
## Run tests for the lookup function
$(MAKE) -C functions/src/handlers/lookup tests
Comment on lines 15 to +22
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mkdir command for the coverage directory should be checked to avoid unnecessary directory creation if it already exists.

-	mkdir -p coverage
+	[ -d coverage ] || mkdir -p coverage
Commitable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
.PHONY: tests
tests:
## Run tests
mkdir -p coverage
go test -v -race -covermode=atomic -coverprofile=coverage/coverage.out ./...
go tool cover -html=coverage/coverage.out -o coverage/coverage.html
## Run tests for the lookup function
$(MAKE) -C functions/src/handlers/lookup tests
.PHONY: tests
tests:
## Run tests
[ -d coverage ] || mkdir -p coverage
go test -v -race -covermode=atomic -coverprofile=coverage/coverage.out ./...
go tool cover -html=coverage/coverage.out -o coverage/coverage.html
## Run tests for the lookup function
$(MAKE) -C functions/src/handlers/lookup tests


docker-compose:
docker compose up -d mysql
Expand Down
8 changes: 4 additions & 4 deletions config/tarmac.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@
"name": "airport-lookup",
"functions": {
"init": {
"filepath": "/functions/init.wasm"
"filepath": "/functions/data/init.wasm"
},
"load": {
"filepath": "/functions/load.wasm"
"filepath": "/functions/data/load.wasm"
},
"fetch": {
"filepath": "/functions/fetch.wasm"
"filepath": "/functions/data/fetch.wasm"
},
"handler": {
"filepath": "/functions/handler.wasm"
"filepath": "/functions/handlers/lookup.wasm"
}
},
"routes": [
Expand Down
4 changes: 4 additions & 0 deletions functions/src/data/fetch/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
/*
This function fetches airport data from a remote CSV file via HTTP and returns it to the application. It is designed
for reuse throughout the application.
*/
Comment on lines +1 to +4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment block provides a good overview of the purpose of the fetch function. However, it should also mention the expected format of the data (CSV) and any assumptions made (e.g., the structure of the CSV file, if any).

package main

import (
Comment on lines +1 to 7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This review was outside of the patch, but no patch was found that overlapped with it. Original lines [15-15]

The Function struct is missing documentation. It's important to explain the purpose of each member, especially tarmac, which seems to be central to the operation of this function.

Note: This review was outside of the patch, but no patch was found that overlapped with it. Original lines [24-24]

The error message "failed to get airports.csv" is repeated. It would be better to define this as a constant or to use a more descriptive message in each case to differentiate between the error of the HTTP GET request and the error of an unsatisfactory status code.

Note: This review was outside of the patch, but no patch was found that overlapped with it. Original lines [31-31]

The check for rsp.StatusCode >= 299 is unconventional. Typically, the check is for rsp.StatusCode >= 400 to catch client and server errors. If the intention is to catch redirects as well, this should be explicitly documented.

Note: This review was outside of the patch, but no patch was found that overlapped with it. Original lines [37-45]

Error handling is missing if the SDK initialization fails. The application should log the error and exit with a non-zero status code to indicate failure.

	if err != nil {
+		log.Fatalf("Failed to initialize Tarmac SDK: %v", err)
-		return
	}

Expand Down
2 changes: 1 addition & 1 deletion functions/src/handler/go.mod → functions/src/data/init/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module github.com/tarmac-project/example-airport-lookup-go/functions/src/handler
module github.com/tarmac-project/example-airport-lookup-go/functions/src/data/init

go 1.21

Expand Down
0 functions/src/handler/go.sum → functions/src/data/init/go.sum
View file
Open in desktop
File renamed without changes.
5 changes: 5 additions & 0 deletions functions/src/init/main.go → functions/src/data/init/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/*
This function executes on startup to verify the database setup and load initial airport data.

The service will not start until the database is setup and the initial data is loaded.
*/
package main

import (
Expand Down
7 changes: 7 additions & 0 deletions functions/src/data/load/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
/*
The purpose of this function is to download the CSV data (by calling another function), parse it, enrich the data, and
load the contents within the SQL database.

This function is called multiple times throughout the application. It's called by an "init" function and also set
as a scheduled task by itself.
*/
package main

import (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Syntax error in the import block.

import (
+ )
Commitable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
import (
import (
)

Expand Down
3 changes: 3 additions & 0 deletions functions/src/handlers/lookup/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
.PHONY: tests
tests:
go test -v -race ./...
10 changes: 10 additions & 0 deletions functions/src/handlers/lookup/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module github.com/tarmac-project/example-airport-lookup-go/functions/src/handlers/lookup

go 1.21

require (
github.com/tarmac-project/tarmac/pkg/sdk v0.5.0
github.com/valyala/fastjson v1.6.4
)

require github.com/wapc/wapc-guest-tinygo v0.3.3 // indirect
0 functions/src/init/go.sum → functions/src/handlers/lookup/go.sum
View file
Open in desktop
File renamed without changes.
9 changes: 9 additions & 0 deletions functions/src/handler/main.go → functions/src/handlers/lookup/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/*
This function is a lookup request handler function. When a request is received, this function is called and will parse
the incoming request, look up the requested airport from the cache, on cache miss, find the requested data in the
database, and provide it back as a response to the request.
*/
package main

import (
Expand All @@ -8,10 +13,14 @@ import (
"html"
)

// Function is the main function object that will be initialized
// and called by the Tarmac SDK.
type Function struct {
tarmac *sdk.Tarmac
}

// Handler is the entry point for the function and will be called
// when a request is received.
func (f *Function) Handler(payload []byte) ([]byte, error) {
// Parse the incoming request
lc := fastjson.GetString(payload, "local_code")
Expand Down
0 functions/src/handler/main_test.go → functions/src/handlers/lookup/main_test.go
View file
Open in desktop
File renamed without changes.
10 changes: 0 additions & 10 deletions functions/src/init/go.mod
View file
Open in desktop

This file was deleted.