-
Notifications
You must be signed in to change notification settings - Fork 18
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.
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
Deployment Logs #93
Deployment Logs #93
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super cool 😎
type DaemonRequest struct { | ||
Stream bool `json:"stream"` | ||
Repo string `json:"repo,omitempty"` | ||
Container string `json:"container,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As handy as this JSON language is, it feels like a smell to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm what do you mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It just feels like it got hacked onto the language. Its very handy, but something feels strange about it. I'm just murmuring out loud, ignore me 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true, functionality-wise I think it's quite nice though 😛
daemon/handler.go
Outdated
// Run daemon on port | ||
log.Println("Serving daemon on port " + port) | ||
mux := http.NewServeMux() | ||
// Example usage of `authorized' decorator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can kill my old silly comments, think it’s clear what’s goin down here now😜
* New logger class and functions * Migrated "--stream"-supported handlers to new logger * Removed daemon dependency on pkg "log" due to datetime duplication when reporting container logs
⌛ Status: Ready
🎟️ Ticket(s): Closes #37
👷 Changes
inertia local logs --stream
, or access most recent logs withinertia local logs
inertia local logs docker-compose
orinertia local logs mycontainer
specifically-Reworked logging,
though this is bugged right now:https://github.com/ubclaunchpad/inertia/blob/rob/%2337-deployment-logs/daemon/handler.go#L196 doesn't seem to properly return the error to the client - I think its still floating around somewhere because if I runinertia local up
again, the line that was supposed to be returned shows up in the container logs alongside the first log from the second callFIXED as of 754d625, turns out a
newline
is needed for the flush to trigger🔦 Testing Instructions