[draft] HTTP server with connection to database #247
Conversation
Signed-off-by: Kevin H. Luu <kevin@anyscale.com>
reefd/main.go
Outdated
|
|
||
| // a function to connect to database given a path to .db file | ||
| func connectToDatabase(dbPath string) (*sql.DB, error) { | ||
| db, err := sql.Open("sqlite3", dbPath) |
There was a problem hiding this comment.
return sql.Open("sqlite3", dbPath)
There was a problem hiding this comment.
removed this func and used sql.Open("sqlite3", dbPath) instead
reefd/main.go
Outdated
| "encoding/json" | ||
| ) | ||
|
|
||
| type MachineConfigInput struct { |
There was a problem hiding this comment.
don't need to cap MachineConfigInput. it can be just machineConfigInput.
starting with cap means it is "public"
reefd/main.go
Outdated
| Configuration string `json:"configuration"` | ||
| } | ||
|
|
||
| type MachineConfig struct { |
There was a problem hiding this comment.
same here: machineConfig.
| @@ -0,0 +1,124 @@ | |||
| package main | |||
There was a problem hiding this comment.
you would like to apply go fmt to the file
reefd/main.go
Outdated
| rows, err = db.Query("SELECT id, name, configuration FROM machine_config") | ||
| } | ||
| if err != nil { | ||
| http.Error(w, err.Error(), http.StatusInternalServerError) |
There was a problem hiding this comment.
you do not want to return internal sql errors directly to user.
There was a problem hiding this comment.
done. I send the sql errors to log instead and only mention internal error
reefd/main.go
Outdated
| defer rows.Close() | ||
|
|
||
| // Iterate through the queried rows and print out | ||
| var machineConfigs []MachineConfig |
There was a problem hiding this comment.
use []*MachineConfig. this avoids copy.
in general, use struct pointers for most cases.
reefd/main.go
Outdated
| http.Error(w, err.Error(), http.StatusInternalServerError) | ||
| return | ||
| } | ||
| for _, machineConfig := range machineConfigs { |
There was a problem hiding this comment.
use single char var name for machineConfig, like c.
reefd/main.go
Outdated
| err := json.NewDecoder(r.Body).Decode(config) | ||
| if err != nil { |
There was a problem hiding this comment.
if err := xxxx ; err != nil {
}
this limits the scope of err
reefd/main.go
Outdated
| func main() { | ||
| dbPath := flag.String("db", "reef.db", "Path to .db file") | ||
| flag.Parse() | ||
| if _, err := os.Stat(*dbPath); os.IsNotExist(err) { |
There was a problem hiding this comment.
you want to handle other types of error too; don't drop error
There was a problem hiding this comment.
if _, err := os.Stat(*dbPath); err != nil {
if os.IsNotExist(err) {
log.Fatal("not exist")
} else {
log.Fatalf("stat %q db file: %s", *dbPath, err)
}
}
reefd/main.go
Outdated
| configuration string | ||
| } | ||
|
|
||
| // a function to connect to database given a path to .db file |
There was a problem hiding this comment.
// connectToDatabase connects to database ...
reefd/main.go
Outdated
| } | ||
| }) | ||
| log.Printf("Starting server") | ||
| http.ListenAndServe(":1234", nil) |
There was a problem hiding this comment.
handle the err being returned.
| } | ||
| }) | ||
| log.Printf("Starting server") | ||
| if err := http.ListenAndServe(":1234", nil); err != nil { |
There was a problem hiding this comment.
make listen address a flag please?
and maybe use 8000 or 8080 or 80xx as the default port.
| } | ||
|
|
||
| // handleGetRequest processes GET request to /machine/configuration that query the machine config based on name | ||
| func handleGetRequest(db *sql.DB, w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
could you define a server struct/class, with db wrapped in it? and make this a method of that server?
GET /machine/configurationandPOST /machine/configurationcurl -X POST -H "Content-Type: application/json" -d '{"name":"machine3","configuration":"config3"}' http://localhost:1234/machine/configurationcurl -X GET http://localhost:1234/machine/configurationcurl -X GET http://localhost:1234/machine/configuration?name=machine3