Closed laghee closed 5 years ago
@karlcow So... there's something wrong. I suspect it has something to do with context again. I deployed for a few minutes, and the endpoint just hangs there without returning anything so I'm assuming that's a query failure. But I'm not sure...
Do the logs have anything illuminating for around 20h00-20h20 UTC 2/26??
If it's a context issue, I have to figure out how to add context inside the app factory, which seems... meta. I'll go see if I can find out something concrete.
EDIT: Wait, I forgot about the params! It's not responding because I'm trying to view a naked endpoint without any params! 🤦♀️ Hang on...
Nope. Adding params didn't help.
Aside from getting me a 500 - "Internal Server Error" in big ugly html.
Hrmph.
If you're willing/able to give me "operate" access so I can see logs, @karlcow, I'll stop harassing you to look at them... 😈 (Not sure if Moz lets you do that with outside contribs.)
Relevant logs:
So this is interesting -- it actually sends back a successful 200 response when requesting parameters not in the database, but sending a request for dates the data actually covers... fails with this "expecting bytes" issue...
💡 Oh, wait! This is because we're not pulling pre-packaged json from our hacked-up apis anymore! I forgot to convert the list of dict
s to json when sending the response. 🙄 New commit incoming...
OK, @karlcow ... new commit to convert to json ... either I fixed all the things ... or uncovered EXCITING NEW errors! 😆
[I also took out the app_context again because assuming I understood the docs, views are covered automatically, which makes sense and keeps things from getting too meta.]
EDIT: Oooh! It works! I didn't add the "about" and "date_format" fields of the json coming from your original endpoint (it's just the "timeline" payload) -- should they be included eventually?
Just rebased to include the app.json that Heroku needs to make the review app work... I forgot you'd enabled that -- nifty!
Huh, interesting. The PR-93-branch-specific review app logs:
Looks like maybe you can't connect to the db from two deployments at once? I suppose that's a pretty reasonable safety mechanism. (It did work when I deployed this branch directly to production for a few minutes, and all I changed after that was rebasing in the app.json
file.)
EDIT: Ah! Nope -- review apps generate their own separate database!
I just noticed and fixed a rogue space, which screws up the hour format (as shown below), that somehow slipped into the logging config:
Thanks @laghee !!! airport review and merge.
I learned a couple of things about timestamp defaults by messing around on a python heroku console (my new bff) that may prove important to remember going forward:
Since DailyTotals days are saved a python datetimes, they automatically get a midnight timestamp
Time is taken into account when filtering SQL with the "BETWEEN" command. Between is inclusive, but if you don't specify a time... SQL assumes midnight (00:00:00). This means that if you search between, say, Feb. 25 and Feb. 26, without specifying a time -- you get back nothing from Feb. 26 that happened after the stroke of 24:00... so basically, only results from Feb. 25.
In writing this, I just realized that I forgot to go back and address point num. 2 in this PR. 🤦♀️ New commit coming soon. 😜