Display HTTP URL in /apps list #325
Conversation
levb
added
2: Dev Review
|
Any security issues that may arise from this? Should end-users be able to know where the http server resides? |
Codecov Report
@@ Coverage Diff @@
## master #325 +/- ##
==========================================
- Coverage 29.42% 29.31% -0.12%
==========================================
Files 79 79
Lines 4676 4694 +18
==========================================
Hits 1376 1376
- Misses 3155 3173 +18
Partials 145 145
Continue to review full report at Codecov.
|
|
Sorry just pushed another update, please review again |
@mickmister no - it's a sysadmin-only command. |
| // all ping requests must respond, unreachable respond with "". | ||
| reachableCh := make(chan apps.AppID) | ||
| for _, app := range all { | ||
| rr, cancel := p.timeoutRequest(r, pingAppTimeout) |
There was a problem hiding this comment.
Can we utilize this timeout logic when fetching bindings from all apps?
There was a problem hiding this comment.
Sure, when the time for it comes? I am not sure we want to be timing out more aggressively than the default, yet, since the binding fetch should be in theory async to the user, right?
…dapps into lev-info-improve
levb
added
4: Reviews Complete
| func (p *Proxy) timeoutRequest(r *incoming.Request, timeout time.Duration) (*incoming.Request, context.CancelFunc) { | ||
| r = r.Clone() | ||
| ctx, cancel := context.WithTimeout(r.Ctx(), timeout) | ||
| incoming.WithCtx(ctx)(r) | ||
| return r, cancel | ||
| } |
There was a problem hiding this comment.
Should this be a method of incoming.Request? What does it have to do with Proxy?
Just as the title says, instead of
http, displayhttps:/x.y.zfor installed HTTP apps