[GitHub] [trafficcontrol] rob05c commented on a change in pull request #4268: Fix Traffic Ops Perl endpoints to always be numbers

[GitBox <gi...@apache.org>]

rob05c commented on a change in pull request #4268: Fix Traffic Ops Perl endpoints to always be numbers
URL: https://github.com/apache/trafficcontrol/pull/4268#discussion_r364829502
 
 

 ##########
 File path: traffic_ops/app/lib/API/ServerCheck.pm
 ##########
 @@ -84,7 +84,7 @@ sub read {
 		$v->{revalPending}	= \$server->reval_pending;
 		foreach my $col (qw/aa ab ac ad ae af ag ah ai aj ak al am an ao ap aq ar at au av aw ax ay az ba bb bc bd be bf/) {
 			if ( defined( $mapping{$col} ) && defined( $server->servercheck ) ) {
-				$v->{checks}->{ $mapping{$col} } = $server->servercheck->$col();
+				$v->{checks}->{ $mapping{$col} } = $server->servercheck->$col() + 0;
 
 Review comment:
   Sure, I can do that, to preserve the existing behavior.
   
   It makes me a little worried though, the documentation says nothing about those values possibly being null, and I had no idea they could be null in the db. In fact, I checked it, and the docs don't even say the type, it just has a number in the example. We could change the docs to say it's possibly null. But then that could cause issues if we decide to make it always a number when we convert it to Go. But, I have no idea who is using this endpoint for what, and if we'd break anyone making nulls in the db become 0's.
   
   So, I don't know what the right answer is, for docs or code. But IMO we should try to remember to revisit it when we rewrite it in Go.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services