Updates to work with client in tc-tools #8
Conversation
ydidwania
changed the title
src/api.js
Outdated
| @@ -88,16 +93,21 @@ builder.declare({ | |||
| res.writeHead(200, { | |||
| 'Content-Type': 'text/event-stream', | |||
| 'Cache-Control': 'no-cache', | |||
| 'Access-Control-Allow-Origin': '*', | |||
There was a problem hiding this comment.
Do we need this line? taskcluster-lib-api seems to set this header here.
src/api.js
Outdated
|
|
||
| listener.resume().then(() => { | ||
| sendEvent('ready'); | ||
| idleTimeout = setTimeout(() => abort(idleMessage), 20*1000); |
There was a problem hiding this comment.
L114 seems to be creating a similar timeout. Could we have those places call createIdleTimeout instead to remove duplication? Also this will ensure we don't forget to clear the timeout before creating a new one.
There was a problem hiding this comment.
Yeah. Also I was thinking of increasing the timeout to 120seconds. 20s seems too short to me now.
src/api.js
Outdated
| @@ -139,6 +149,10 @@ builder.declare({ | |||
| sendEvent('error', errorMessage); | |||
| } finally { | |||
|
|
|||
| if (idleTimeout) { | |||
| clearInterval(idleTimeout); | |||
| @@ -36,6 +36,3 @@ test: | |||
|
|
|||
| server: | |||
| port: 12345 | |||
|
|
|||
There was a problem hiding this comment.
We should not permanently remove this. Just remove it locally when you want to rest for real.
There was a problem hiding this comment.
test/helper.js injects fake : true anyway, so I thought we shouldn't be needing this here
There was a problem hiding this comment.
Ah, great. Then I retract my statement.
src/api.js
Outdated
| @@ -15,6 +15,9 @@ let builder = new APIBuilder({ | |||
| serviceName: 'events', | |||
| version: 'v1', | |||
| context: ['listeners'], | |||
| errorCodes: { | |||
| NoReconnects: 204, // Not supporting automatic reconnects from EventSource | |||
There was a problem hiding this comment.
Nit: Remove extra white space between the column and 204.
Attempting to set up a connection using EventSource in the browser throws an error in the console:
Request header field Cache-Control is not allowed by Access-Control-Allow-Headers in preflight response..Thus
tc-lib-apiwas updated to v12.3.0 to addCache-ControltoAccess-Control-Allow-HeadersAlso I removed
from
config.ymlto allow me to run the server onlocalhostwith real pulse creds inuser-config.yml