[MediaWiki-commits] [Gerrit] Refactored logging and a few other things per review - change (mediawiki...graphoid)
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 yu...@wikimedia.org
Gerrit-Reviewer: Mobrovac mobro...@wikimedia.org
Gerrit-Reviewer: Yurik yu...@wikimedia.org
[MediaWiki-commits] [Gerrit] Refactored logging and a few other things per review - change (mediawiki...graphoid)
Yurik has uploaded a new change for review.
https://gerrit.wikimedia.org/r/202910
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(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/graphoid
refs/changes/10/202910/1
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: newchange
Gerrit-Change-Id: Ia86ac8076ddab043e7e1207da5cc7093d25efe0d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/graphoid
Gerrit-Branch: master
Gerrit-Owner: Yurik yu...@wikimedia.org
___
MediaWiki-commits mailing list
