-
Notifications
You must be signed in to change notification settings - Fork 29
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
Fiqare perseo-fe improvements #419
Changes from 107 commits
c620a0a
b1eb8fa
d38ffcc
f18f17b
fb9cb75
d9d800d
e666027
5f799c9
460c75f
24bbb3f
c87e96f
26a081e
9fdc5b9
8200b45
15c1d5d
2e81912
a514588
b8b3ce2
ca2b66c
ea6a05d
aa23fed
05ac1f4
88344c8
2e1025f
e75d07d
2cd53b5
a3c1cc1
c012cdc
326a6b4
9e873b8
9b15277
0695fef
c0b8505
c11d45d
05a5208
e895b69
590de2a
53c6052
eedd341
8bda3a7
e4e9bca
01ec831
805219b
32662f4
4cade58
14a0867
ef334a6
07c164e
1c894b0
e6b731a
5d3fbc4
862b176
c68e52d
6f64b09
334a59e
6a10db1
e67403a
fe26c90
70da2e4
8ab124b
3580b61
7f7ad60
4a5b42e
fd6e450
b54bc43
c5a694f
6f05372
9a77dbb
5440640
41f9c68
75aa10d
f471ce2
14b1aa9
e090738
4db0223
ec05fb7
1bfb721
13089b0
88ad953
dd56330
e71c7e6
97bc1f3
c08843e
94fa819
57334b9
9a0e631
9e975a9
27716c8
c6604b0
f3a905a
c1cddf2
e622aa3
70a972d
bd3c7b2
aa0df1d
ac5620a
4e300a4
e8d11ac
42267ba
6e3974e
08201e7
3020aab
51271cd
ce583be
d978818
ff07d10
0865dc8
8371d53
9c80eb9
df81a41
210813b
c29e89f
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 |
---|---|---|
|
@@ -25,13 +25,12 @@ | |
var util = require('util'), | ||
nodemailer = require('nodemailer'), | ||
logger = require('logops'), | ||
smtpTransport = require('nodemailer-smtp-transport'), | ||
config = require('../../config'), | ||
myutils = require('../myutils'), | ||
alarm = require('../alarm'), | ||
metrics = require('./metrics'); | ||
|
||
var transporter = nodemailer.createTransport(smtpTransport(config.smtp)); | ||
var transporter = nodemailer.createTransport(config.smtp); | ||
Comment on lines
-34
to
+33
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. Why this change (along with the removal in L28) is actually needed? 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 function createTransport accepts the configuration as a string, so the deleted var in L28 was no necessary and redundant. 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. Ok. NTC. |
||
|
||
function buildMailOptions(action, event) { | ||
return { | ||
|
@@ -50,7 +49,7 @@ function SendMail(action, event, callback) { | |
|
||
metrics.IncMetrics(event.service, event.subservice, metrics.actionEmail); | ||
|
||
transporter.sendMail(mailOptions, function(err, info) { | ||
transporter.sendMail(mailOptions, (err, info) => { | ||
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. I understand the preferred code style is currently 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. Fixed in 9c80eb9 |
||
logger.debug('emailAction.SendMail %j %j %j', mailOptions, err, info); | ||
// Not an HTTP request, so outgoingTransacion hasn't already counted and must be counted now | ||
metrics.IncMetrics(event.service, event.subservice, metrics.outgoingTransactions); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,9 @@ function findSilentEntities(service, subservice, ruleData, func, callback) { | |
} catch (e) { | ||
return callback(e, null); | ||
} | ||
} else { | ||
//Added default else clause | ||
logger.debug('findSilentEntities() - Default else clause'); | ||
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. Review indentation please ( 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. I checked the last commit and I can't locate the indentation error 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. 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. Fixed in 0865dc8 |
||
} | ||
if (ruleData.type) { | ||
criterion['_id.type'] = ruleData.type; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,10 @@ function buildPostOptions(action, event) { | |
} else if (action.template) { | ||
options.text = myutils.expandVar(action.template, event, true); | ||
} | ||
else { | ||
//Added default else clause | ||
return options; | ||
} | ||
|
||
return options; | ||
} | ||
|
@@ -58,6 +62,10 @@ function doIt(action, event, callback) { | |
} else if (options.text) { | ||
requestOptions.body = options.text; | ||
} | ||
else { | ||
//Added default else clause | ||
logger.debug('doIt() - Default else clause'); | ||
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. Review indent. 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. Fixed in d978818 |
||
} | ||
|
||
metrics.IncMetrics(event.service, event.subservice, metrics.actionHttpPost); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,7 +33,8 @@ var util = require('util'), | |
actions = require('./actions'), | ||
namePattern = /^[a-zA-Z0-9_-]+$/, | ||
MaxNameLength = 50, | ||
errors = {}; | ||
errors = {}, | ||
logger = require('logops'); | ||
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. Better to have all the require() together. Pleace move up (just afeter 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. Fixed in 9c80eb9 |
||
|
||
function validNSRule(rule) { | ||
var checkInterval = parseInt(rule.nosignal.checkInterval, 10); | ||
|
@@ -58,6 +59,9 @@ function validRule(rule) { | |
return new errors.InvalidRuleName(rule.name); | ||
} else if (!rule.text && !rule.nosignal) { | ||
return new errors.EmptyRule(JSON.stringify(rule)); | ||
} else { | ||
//Added default else clause | ||
logger.debug('validRule() - Default else clause'); | ||
} | ||
if (rule.nosignal) { | ||
//Specific checks for no-signal rules | ||
|
@@ -218,6 +222,7 @@ module.exports = { | |
}, | ||
Save: function(rule, callback) { | ||
var localError; | ||
|
||
localError = validRule(rule); | ||
if (localError !== null) { | ||
myutils.logErrorIf(localError); | ||
|
@@ -244,7 +249,12 @@ module.exports = { | |
rulesStore.Save.bind(null, rule), | ||
function(cb) { | ||
if (rule.nosignal) { | ||
noSignal.AddNSRule(rule.service, rule.subservice, rule.name, rule.nosignal); | ||
noSignal.AddNSRule( | ||
rule.service, | ||
rule.subservice, | ||
rule.name, | ||
rule.nosignal | ||
); | ||
} | ||
cb(null); | ||
} | ||
|
@@ -294,14 +304,23 @@ module.exports = { | |
delR2core.bind(null, rule), | ||
function(cb) { | ||
if (rule.nosignal) { | ||
noSignal.DeleteNSRule(rule.service, rule.subservice, rule.name); | ||
noSignal.DeleteNSRule( | ||
rule.service, | ||
rule.subservice, | ||
rule.name | ||
); | ||
} | ||
cb(null); | ||
}, | ||
postR2core.bind(null, rule), | ||
function(cb) { | ||
if (rule.nosignal) { | ||
noSignal.AddNSRule(rule.service, rule.subservice, rule.name, rule.nosignal); | ||
noSignal.AddNSRule( | ||
rule.service, | ||
rule.subservice, | ||
rule.name, | ||
rule.nosignal | ||
); | ||
} | ||
cb(null); | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -97,7 +97,7 @@ function search(rule, callback) { | |
|
||
function findAll(service, subservice, callback) { | ||
var criterion = {}; | ||
|
||
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. Remove this extra whitespace, pls. 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. Fixed in 210813b |
||
if (typeof service === 'function') { | ||
callback = service; | ||
} else { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -188,8 +188,9 @@ function doRequest(action, event, token, callback) { | |
} | ||
if (!err && data.body && data.body.orionError) { | ||
err = new errors.OrionError(JSON.stringify(data.body.orionError)); | ||
} else { | ||
callback(err, data); | ||
} | ||
callback(err, data); | ||
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. I think this change is not correct... the callback() needs to be changed in any case and, after this change, it is only called when the execution flow goes by the "else" part. 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. Fixed in c29e89f |
||
} | ||
); | ||
} | ||
|
@@ -266,6 +267,8 @@ function processOptionParams(action, event) { | |
case 'None': | ||
theValue = null; | ||
break; | ||
default: | ||
//Nothing to do | ||
} | ||
} | ||
var key = myutils.expandVar(attr.name, event); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,7 +48,9 @@ var domain = require('domain'), | |
serviceMiddleware = require('./middleware/service'), | ||
errorMiddleware = require('./middleware/error'), | ||
myutils = require('./myutils'), | ||
d = domain.create(); | ||
d = domain.create(), | ||
swaggerJsdoc = require('swagger-jsdoc'), | ||
swaggerUi = require('swagger-ui-express'); | ||
|
||
function start(callbackStart) { | ||
var context = { op: 'start', comp: constants.COMPONENT_NAME }; | ||
|
@@ -98,6 +100,22 @@ function start(callbackStart) { | |
versionRoutes.AddTo(app); | ||
metricsRoutes.AddTo(app); | ||
|
||
const options = { | ||
definition: { | ||
swagger: '2.0', // Specification (optional, defaults to swagger: '2.0') | ||
info: { | ||
title: 'Perseo Front-End', // Title (required) | ||
version: '1.7.0-fiqare' // Version (required) | ||
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. This should be got from packages.json 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. Fixed in 9c80eb9 |
||
} | ||
}, | ||
// Path to the API docs | ||
apis: ['./lib/routes/*.js'] | ||
}; | ||
|
||
const specs = swaggerJsdoc(options); | ||
|
||
app.use('/api-docs', swaggerUi.serve, swaggerUi.setup(specs)); | ||
|
||
async.series( | ||
[ | ||
function(callback) { | ||
|
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.
CHANGES_NEXT_RELEASE entries should be added regarding the changes in this documentation. Suggestion:
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.
Fixed in 210813b