devices handler #23
devices handler #23
Conversation
radsec
force-pushed
the
fix_readme_and_enrollment_endpoint
branch
from
658b596
to
9c360c9
Compare
| dsn string | ||
| ) | ||
|
|
||
| func main() { |
There was a problem hiding this comment.
This main() should remain here in main.go under devices
There was a problem hiding this comment.
it is under /devices? I'm not sure what you mean.
| if err != nil { | ||
| log.Fatal(err) | ||
| } | ||
| rows, err := db.Query(`select * from enrollments;`) |
There was a problem hiding this comment.
Can this be migrated to the internal folder if this logic is to only remain here....keeps the main function clean as it just calls the executing functions that are required. --> internal/devices module
There was a problem hiding this comment.
Fair point. will work on this.
| log.Fatal(err) | ||
| } | ||
| defer rows.Close() | ||
| var devices []DeviceDTO |
There was a problem hiding this comment.
Would also create a types for all these under that new internal/devices module to keep this cleaner
| LastSeen: lsa, | ||
| DEPProfileStatus: "unknown", | ||
| } | ||
| type Serial struct { |
There was a problem hiding this comment.
Same with the types suggestion, migrate all of these to a types folder or try to consolidate types if possible for readability.
There was a problem hiding this comment.
I kind of disagree that Serial is worthy of being stuffed in another package somewhere. It's such a small struct that is only used here, in-line. I also don't really like the types pattern as I'd prefer to organize by business logic instead of go type.
| devices = append(devices, ddto) | ||
| } | ||
|
|
||
| devicesHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
Would create a separate handlers for this. Honestly, the handler that I refactored in enroll_endpoint is very simple and allows us to add a quick logger and is pretty easy to understand the routing. Since the main purpose of this is to serve as a HTTP server, it is very clear then it should operate like that vs a CLI tool that offers up an HTTP server interface
There was a problem hiding this comment.
Please clarify your comment "would create a separate handlers" for this. I'm not sure what you mean.
No description provided.