Draft: Add fridge stats and S3 upload #55
Conversation
1jeanpaul1
left a comment
1jeanpaul1
left a comment
There was a problem hiding this comment.
Function looks good! Thank you :)
This wasn't part of the ticket but do you want to try to make it into a GET api? You'd have to create a lambda handler. Under functions you can create a new folder fridge_stats. Look for if httpMethod == "GET": to get a reference on how to do it.
If you want to be able to test the api you will have to update the template.yaml file with a new resource, you can look at FridgeReportFunction for reference
the api path could be: /v1/fridges/stats
can look a bit confusing if you've never worked with cloudformation templates before, let me know if you need help and I can explain what each thing does
|
|
||
| stats[condition]+=1 | ||
| else: | ||
| stats[condition] = 1 |
There was a problem hiding this comment.
this would be useful in the case that we have a new condition?
I think if we have a new condition we might want to be explicit about it and add it to the stats map
What do you think about using FridgeReport.VALID_CONDITIONS to generate the stats map? And for the case where the condition is not in stats, I think still leave it as it, but maybe add a log? "Warning: unknown condition"
What do you think?
| message="Tag was succesfully added", status_code=200, success=True | ||
| ) | ||
|
|
||
| def get_fridge_stats(self): |
There was a problem hiding this comment.
We should put this function in the Fridge class?
There was a problem hiding this comment.
right now this function is in the Tag class? It should go in the Fridge Class
There was a problem hiding this comment.
Could you write some unit tests for this function?
| # edit: using fridgereport.valid conditions for stat map | ||
| stats = {condition: 0 for condition in FridgeReport.VALID_CONDITIONS} | ||
| stats["total"] = 0 | ||
| stats["no report"] = 0 |
There was a problem hiding this comment.
could we name it no_report?
I know our valid_conditions uses spaces instead of underscores, but we really shouldn't be using spaces 😭. Maybe a ticket for the future would be to refactor the naming to use underscores, but not a big deal right now
|
|
||
| def get_fridge_stats(self): | ||
|
|
||
| response = self.db_client.scan(TableName = self.TABLE_NAME) |
There was a problem hiding this comment.
We will eventually want to paginate this scan, up to you if you want to paginate but this works for now :)
| condition = latestFridgeReport["condition"].lower(); | ||
| if (condition in stats): | ||
| stats[condition]+=1 | ||
| elif condition: # edit: if new condition, then we want to set to 1. |
There was a problem hiding this comment.
after thinking about it some more, I don't feel good about returning invalid conditions
What do you think about raising the warning from logger.warning to logger.error? and using stats['error'] or stats['unknown'] as our counter for unknown conditions?
There was a problem hiding this comment.
note: there really shouldn't be any invalid cases, but if there are this is a good place to catch them
3 participants
Implements fridge stats data collection + writing to s3