Fixed issue with new bills not grabbing data #3404

Merged
merged 1 commit into from Apr 29, 2016

Projects

None yet

4 participants

@laf
Member
laf commented Apr 21, 2016

@richardlawley would you have a quick check on this pls.

We had a user saying new bills weren't collecting data, this patch fixed it for them (existing bills were ok as data was returned from the query and used).

@laf laf Fixed issue with new bills not grabbing data
bc26c91
@laf laf added Bug Billing labels Apr 21, 2016
@richardlawley
Contributor

I'm not sure how this could've happened. The fix hasn't rolled out to my install, and I've added a new bill without getting that error. Checking the flow through the code it doesn't look like it's possible for that value to be empty.

I can't see it causing a problem, I'm just not sure what it actually fixed!

@laf
Member
laf commented Apr 21, 2016

It was a couple of weeks ago I looked at it for the chap involved but:

$period = dbFetchCell("SELECT UNIX_TIMESTAMP(CURRENT_TIMESTAMP()) - UNIX_TIMESTAMP('".mres($prev_timestamp)."')");

Would return null, so forcing a check that $period actually had data fixed it.

I realise it shouldn't need this but if there is a possibility that the $period value is not set then it stops bills so this fix just seems to safe guard things more than anything.

@richardlawley
Contributor

For that line to return null, $prev_timestamp must be null, but I cannot see a way this can happen. The only thing I can think of is that the is_null check doesn't work here, but it works fine on my system.

If you have a way to reproduce this problem, I'd be interested to see it, as it suggests there is something more complicated going on.

@laf
Member
laf commented Apr 21, 2016

I don't have a way to reproduce as it works as expected for me but not someone else (Dragon2611 on IRC).

@Rosiak
Contributor
Rosiak commented Apr 24, 2016

I would say this is good to merge, afaik this doesn't break things(At least not on my install)

@laf laf merged commit a89b9bc into librenms:master Apr 29, 2016

3 checks passed

Auto-Deploy Build finished. No test results found.
Details
Scrutinizer No new issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@laf laf deleted the laf:billing-fix branch Jan 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment