Closed mduvanel closed 5 years ago
I was going to ask if this PR is ready for merging, but it looks like there is a test failing...
I was going to ask if this PR is ready for merging, but it looks like there is a test failing...
Indeed, and I can reproduce the issue locally. Note that this affects one of the Time
tests and I have a hard time understanding which one of my changes could affect this behaviour. This seems to affect the master branch too:
https://travis-ci.org/sematext/logsene-cli/builds
I will try to understand what is wrong in my local setup though.
After some playing with the tests, this seems to be a bug/limitation in moment.js. I reproduced the misbehaviour in a dedicated test:
var duration = moment.duration({y: 1, M: 2, d: 8, h: 11, m: 6, s: 8});
var now = moment();
var start = now.clone().subtract(duration);
console.log('substracting ' + duration.valueOf() + ' to ' + now.valueOf() + ' = ' + start.valueOf());
var end = start.clone().add(duration);
console.log('adding ' + duration.valueOf() + ' to ' + start.valueOf() + ' = ' + end.valueOf());
expect(end.valueOf()).to.be.closeTo(moment().valueOf(), 100);
Here is an example output on my machine:
substracting 37451168000 to 1549222661867 = 1511598693867
adding 37451168000 to 1511598693867 = 1549136261867
AssertionError: expected 1549136261867 to be close to 1549222661868 +/- 100
at Context.<anonymous> (test/test-time.js:164:35)
Notice that 1549222661867 - 37451168000 = 1511771493867, not 1511598693867. Result is 172800000 off, aka 2 days. Notice also that 1511598693867 + 37451168000 = 1549049861867, not 1549136261867. Result is off again by 86400000, aka 1 day... Interestingly, if I change the duration from {y: 1, M: 2, d: 8, h: 11, m: 6, s: 8} to {M: 1, d: 8, h: 11, m: 6, s: 8}, now the test passes.
Hi @mduvanel, you can remove this test that fails. I'll try to look into the PR late tonight. You tested it - does it work?
Hi @mbonaci, I have updated the PR by changing the duration of the test which should now pass (at least it does locally). Regarding testing, here is what I tested:
Login using EU account:
$> ./bin/logsene-cli search
No active sessions. Please log in using your Sematext account:
Enter your region, US or EU [US]: EU
Enter your username: martin.duvanel@XXXXXX
Enter your password:
Successfully logged in and retrieved API key.
Successfuly established Logsene XXX application session.
ES stream had an error or closed early.
$>
Login using US account:
$> ./bin/logsene-cli search error
No active sessions. Please log in using your Sematext account:
Enter your region, US or EU [US]:
Enter your username: martin.duvanel@YYYYYY
Enter your password:
Successfully logged in and retrieved API key.
Successfuly established Logsene YYY application session.
ES stream had an error or closed early.
$>
I also checked that logging in to another region works after session timeout.
Yes, I get the same thing.
It happens due to the obsolete _cache
parameter:
node ./index.js search -q error --trace ~/Sematext/repos/logsene-cli
No active sessions. Please log in using your Sematext account:
Enter your region, US or EU [US]:
Enter your username: marko.bonaci@sematext.com
Enter your password:
Successfully logged in and retrieved API key.
Successfuly established Logsene mbo-us application session.
Elasticsearch INFO: 2019-02-04T22:53:58Z
Adding connection to https://logsene-receiver.sematext.com/
Elasticsearch TRACE: 2019-02-04T22:53:59Z
-> POST https://logsene-receiver.sematext.com:443/6758524b-xxxx-xxxx-xxxx-623c01764053/_search?from=0&size=50&ignore_unavailable=true
{
"query": {
"bool": {
"filter": {
"range": {
"@timestamp": {
"gte": "2019-02-04T21:53:58.283Z"
},
"_cache": false
}
},
"must": {
"query_string": {
"query": "error",
"default_operator": "OR"
}
}
}
},
"sort": [
{
"@timestamp": "asc"
}
]
}
<- 400
{
"error": {
"root_cause": [
{
"type": "parsing_exception",
"reason": "[range] query does not support [_cache]",
"line": 1,
"col": 202
}
]
},
"status": 400
}
Elasticsearch DEBUG: 2019-02-04T22:53:59Z
Request complete
ES stream had an error or closed early.
OK, I removed the _cache
payload param and it now works.
Thank you for your contribution.
I'm merging.
As discussed in #6, support for EU logins is provided by asking for the region at login time, and keeping the setting for the duration of the session. I ran the unit tests to verify they still pass, but haven't written any new ones to verify my changes. I am open to suggestions regarding how to write meaningful tests for these changes!