-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
UI: Take advantage of client request tunneling #3908
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,11 @@ import { inject as service } from '@ember/service'; | |
import Component from '@ember/component'; | ||
import { computed } from '@ember/object'; | ||
import { run } from '@ember/runloop'; | ||
import RSVP from 'rsvp'; | ||
import { task } from 'ember-concurrency'; | ||
import { logger } from 'nomad-ui/utils/classes/log'; | ||
import WindowResizable from 'nomad-ui/mixins/window-resizable'; | ||
import timeout from 'nomad-ui/utils/timeout'; | ||
|
||
export default Component.extend(WindowResizable, { | ||
token: service(), | ||
|
@@ -14,6 +16,15 @@ export default Component.extend(WindowResizable, { | |
allocation: null, | ||
task: null, | ||
|
||
// When true, request logs from the server agent | ||
useServer: false, | ||
|
||
// When true, logs cannot be fetched from either the client or the server | ||
noConnection: false, | ||
|
||
clientTimeout: 1000, | ||
serverTimeout: 5000, | ||
|
||
didReceiveAttrs() { | ||
if (this.get('allocation') && this.get('task')) { | ||
this.send('toggleStream'); | ||
|
@@ -37,11 +48,12 @@ export default Component.extend(WindowResizable, { | |
|
||
mode: 'stdout', | ||
|
||
logUrl: computed('allocation.id', 'allocation.node.httpAddr', function() { | ||
logUrl: computed('allocation.id', 'allocation.node.httpAddr', 'useServer', function() { | ||
const address = this.get('allocation.node.httpAddr'); | ||
const allocation = this.get('allocation.id'); | ||
|
||
return `//${address}/v1/client/fs/logs/${allocation}`; | ||
const url = `/v1/client/fs/logs/${allocation}`; | ||
return this.get('useServer') ? url : `//${address}${url}`; | ||
}), | ||
|
||
logParams: computed('task', 'mode', function() { | ||
|
@@ -51,9 +63,23 @@ export default Component.extend(WindowResizable, { | |
}; | ||
}), | ||
|
||
logger: logger('logUrl', 'logParams', function() { | ||
const token = this.get('token'); | ||
return token.authorizedRequest.bind(token); | ||
logger: logger('logUrl', 'logParams', function logFetch() { | ||
// If the log request can't settle in one second, the client | ||
// must be unavailable and the server should be used instead | ||
const timing = this.get('useServer') ? this.get('serverTimeout') : this.get('clientTimeout'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the added flip flopping is nice 🙆♂️ |
||
return url => | ||
RSVP.race([this.get('token').authorizedRequest(url), timeout(timing)]).then( | ||
response => response, | ||
error => { | ||
if (this.get('useServer')) { | ||
this.set('noConnection', true); | ||
} else { | ||
this.send('failoverToServer'); | ||
this.get('stream').perform(); | ||
} | ||
throw error; | ||
} | ||
); | ||
}), | ||
|
||
head: task(function*() { | ||
|
@@ -100,5 +126,8 @@ export default Component.extend(WindowResizable, { | |
this.get('stream').perform(); | ||
} | ||
}, | ||
failoverToServer() { | ||
this.set('useServer', true); | ||
}, | ||
}, | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
import Ember from 'ember'; | ||
import { alias } from '@ember/object/computed'; | ||
import { assert } from '@ember/debug'; | ||
import Evented from '@ember/object/evented'; | ||
|
@@ -10,6 +11,8 @@ import PollLogger from 'nomad-ui/utils/classes/poll-logger'; | |
|
||
const MAX_OUTPUT_LENGTH = 50000; | ||
|
||
export const fetchFailure = url => () => Ember.Logger.warn(`LOG FETCH: Couldn't connect to ${url}`); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ember.Logger's is being deprecated and removed (not that it matters here or will happen soon), but console.warn is likely fine here too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah... Not gonna lie, I just wanted to avoid adding an eslint-ignore. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lol, fair |
||
|
||
const Log = EmberObject.extend(Evented, { | ||
// Parameters | ||
|
||
|
@@ -74,9 +77,9 @@ const Log = EmberObject.extend(Evented, { | |
const url = `${this.get('url')}?${queryParams}`; | ||
|
||
this.stop(); | ||
let text = yield logFetch(url).then(res => res.text()); | ||
let text = yield logFetch(url).then(res => res.text(), fetchFailure(url)); | ||
|
||
if (text.length > MAX_OUTPUT_LENGTH) { | ||
if (text && text.length > MAX_OUTPUT_LENGTH) { | ||
text = text.substr(0, MAX_OUTPUT_LENGTH); | ||
text += '\n\n---------- TRUNCATED: Click "tail" to view the bottom of the log ----------'; | ||
} | ||
|
@@ -96,7 +99,7 @@ const Log = EmberObject.extend(Evented, { | |
const url = `${this.get('url')}?${queryParams}`; | ||
|
||
this.stop(); | ||
let text = yield logFetch(url).then(res => res.text()); | ||
let text = yield logFetch(url).then(res => res.text(), fetchFailure(url)); | ||
|
||
this.set('tail', text); | ||
this.set('logPointer', 'tail'); | ||
|
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.
I still think 1s is ambitious (networking! ¯_(ツ)_/¯ ), but most likely not a huge deal.
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.
I agree. Especially if the browser attempting the connection is on a slow network or is far away from the server, but it's a careful balance between timing out too early and wasting time before just using the server. It's easy to change for 0.8.1 if need be.