Yurik has submitted this change and it was merged.
Change subject: Refactored logging and a few other things per review
......................................................................
Refactored logging and a few other things per review
* Vega is now required directly
* Protocol is now expected not to have a ':'
* Logging fixed
* returns a promise in failOnTImeout
Bug: T95417
Change-Id: Ia86ac8076ddab043e7e1207da5cc7093d25efe0d
---
M routes/v1.js
1 file changed, 10 insertions(+), 29 deletions(-)
Approvals:
Mobrovac: Looks good to me, but someone else must approve
Yurik: Verified; Looks good to me, approved
diff --git a/routes/v1.js b/routes/v1.js
index 92d54ad..1fdf721 100644
--- a/routes/v1.js
+++ b/routes/v1.js
@@ -5,7 +5,7 @@
var domino = require('domino');
var sUtil = require('../lib/util');
var urllib = require('url');
-var vega = null; // Visualization grammar - https://github.com/trifacta/vega
+var vega = require('vega'); // Visualization grammar -
https://github.com/trifacta/vega
/**
* The main router object
@@ -30,7 +30,7 @@
/**
* For protocol-relative URLs (they begin with //), which protocol should we
use
*/
-var defaultProtocol = 'http:';
+var defaultProtocol = 'http';
/**
* Limit request to 10 seconds by default
@@ -87,11 +87,8 @@
* @param domains array of strings - which domains are valid
*/
function initVega(domains) {
- if (!vega) {
- return;
- }
vega.config.domainWhiteList = domains;
- vega.config.defaultProtocol = defaultProtocol;
+ vega.config.defaultProtocol = defaultProtocol + ':';
vega.config.safeMode = true;
vega.config.isNode = true; // Vega is flaky with its own detection, fails
in tests and with IDE debug
@@ -195,7 +192,7 @@
domain2 = (domainMap && domainMap[domain2]) || domain2;
state.domain = domain2;
- state.apiUrl = defaultProtocol + '//' + domain2 + '/w/api.php';
+ state.apiUrl = defaultProtocol + '://' + domain2 + '/w/api.php';
if (domain !== domain2) {
state.log.backend = domain2;
}
@@ -257,7 +254,7 @@
if (res.hasOwnProperty('warnings')) {
state.log.apiWarning = res.warnings;
- log('warn/domain-warning', state.log);
+ state.request.logger.log('warn/domain-warning', state.log);
// Warnings are usually safe to continue
}
@@ -295,16 +292,11 @@
function renderOnCanvas(state) {
return new BBPromise(function (fulfill, reject){
- if (!vega) {
- // If vega is down, keep reporting it
- throw new Err('fatal/vega', 'vega.missing');
- }
-
var start = Date.now();
// BUG: see comment above at vega.data.load.sanitizeUrl = ...
// In case of non-absolute URLs, use requesting domain as "local"
- vega.config.baseURL = defaultProtocol + '//' + state.domain;
+ vega.config.baseURL = defaultProtocol + '://' + state.domain;
vega.headless.render({spec: state.graphData, renderer: 'canvas'},
function (err, result) {
if (err) {
@@ -340,15 +332,15 @@
.then(downloadGraphDef)
.then(renderOnCanvas);
- failOnTimeout(render, timeout)
+ return failOnTimeout(render, timeout)
.then(function () {
// SUCCESS
// For now, record everything, but soon we should scale it back
- log('info/ok', state.log);
+ req.logger.log('info/ok', state.log);
metrics.endTiming('total.time', start);
- },function (reason) {
+ }, function (reason) {
// FAILURE
var l = state.log;
@@ -369,7 +361,7 @@
res.status(400).json(msg);
metrics.increment(mx);
- log(msg, l);
+ req.logger.log(msg, l);
});
});
@@ -383,21 +375,10 @@
log('info/init', 'starting v1' );
metrics.increment('v1.init');
- try{
- // Simplify debugging when vega is not available
- vega = require('vega');
- } catch(err) {
- log('fatal/vega', err);
- }
-
var conf = app.conf;
var domains = conf.domains || domains;
timeout = conf.timeout || timeout;
defaultProtocol = conf.defaultProtocol || defaultProtocol;
- if (!defaultProtocol.endsWith(':')) {
- // colon in YAML has special meaning, allow it to be skipped
- defaultProtocol = defaultProtocol + ':';
- }
var validDomains = domains;
if (conf.domainMap && Object.getOwnPropertyNames(conf.domainMap).length >
0) {
--
To view, visit https://gerrit.wikimedia.org/r/202910
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia86ac8076ddab043e7e1207da5cc7093d25efe0d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/graphoid
Gerrit-Branch: master
Gerrit-Owner: Yurik <[email protected]>
Gerrit-Reviewer: Mobrovac <[email protected]>
Gerrit-Reviewer: Yurik <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
