Use static path for saving local councillor headshot

[patcon]

Partial solution for #48, that at least makes the paths static between database resets

Ready for review. Minimal change that shouldn't break any existing instances (HEADSHOT_PATH and model's headshot_url prop could be DRY'd out, but that was already an issue, and this is no worse)

[patcon]

OK, no longer backward compat, but DRY. Changed the setting name to make it more obvious what sort of path

[patcon]

HEADSHOT_RELPATH should be relative to a static root, ie

HEADSHOT_RELPATH = 'images/headshots'

[cathydeng]

hey @patcon - can you catch me up on what problem this is solving? I'm not sure what parts of #48 this is addressing, & I'm reluctant to make changes that break existing instances of councilmatic w/o fully understanding the justification

[patcon]

Sure!

Happy to take another tack, but the original issue was:

1. In the Canadian scrapers we blow away the database each time, so the person uuids change often.
2. Heroku apps don't have a persistent filesystem, and so it can only collect static images at specific times. With the avatars, the images are collected during our custom loaddata command, which is non-standard (collectstatic being the standard place to generate static files, which heroku expects in order to do it's django-specific caching of static files)

Basically, Canadian Councilmatic instances need a way to store images that is more persistent then uuids (for which slugs are a good candidate), so that we can simply version-control the images for now.

This isn't ideal, but seems the best option to accommodate us until I have time to submit a custom STATICFILES_FINDER that is acceptable to datamade. (This finder would allow the images to be collected during collectstatic in the location/tim that django and heroku expect static files to be generated.)

[cathydeng]

why are you blowing away the database each time you run the scraper? that seems to be the bigger problem

we added the manual headshots functionality so that we can override the default headshot_url for a person & have images under version control. can you explain why the current manual headshots setup isn't sufficient for toronto?

[patcon]

why are you blowing away the database each time you run the scraper? that seems to be the bigger problem

@jpmckinney?

[patcon]

I saw that manual headshot feature, but it was forcing the maintainer to maintain a large dict of all 44 councillors, which seemed unneccessary. I tried to tackle that here, attempting a more dynamic method that didn't need a giant repetitive dict for config: #48

The conversation there tapered off and made me feel it wasn't a welcome approach, so I instead tried to take the angle in this PR, so I wouldn't be adding another location, but simply the location would be persistent across database flushes like the Canadian scrapers require.

[cathydeng]

a big hash of all 44 people isn't ideal if the image sources don't differ (like for chicago), but imo it's not worth breaking existing councilmatic instances to avoid doing so, esp since this problem can also be addressed further upstream (not breaking all ids every time the scraper is run)

agree that the existing headshot handling is kinda awkward tho - we'd be open to PRs that refactor the code without breaking existing instances

[patcon]

we'd be open to PRs that refactor the code without breaking existing instances

Ok, thanks. I think that's doable! I'll let james chime in, as the upstream infrastructure is his, and I'm not clear on the exact rationale

[patcon]

Y'know... this is so easy to override, and this thread is already too long. Let's just make this Toronto-specific :)

[jpmckinney]

OCD doesn't yet have tools that existed in its predecessor Billy to easily handle people leaving office, or to easily handle duplicates (e.g. if the source expands a person's initial or corrects a typo), so instead of writing that tooling, we just delete people before every scrape, because for the Represent API, consistent IDs are not needed. To change this, we'd need to add a process to sweep people who weren't included in the latest scrape. See opencivicdata/pupa#65