harvard-lil / perma

Indelible links
413 stars 70 forks source link

Add global feature flag and scaffolding #3382

Closed rebeccacremona closed 1 year ago

rebeccacremona commented 1 year ago

This is a small PR with a big diff: it's easiest to see commit-by-commit.

It adds a single new setting:

CAPTURE_ENGINE = 'perma'  # perma|scoop-api

It tweaks the structure of the existing run_next_capture Celery task: right at the point where, in main right now, the actual work of doing a capture starts, this PR adds a conditional. If settings.CAPTURE_ENGINE == 'perma', call a helper function that runs all the same code as usual. If settings.CAPTURE_ENGINE == 'scoop-api', call a new placeholder helper function where we can start adding the new capture code. Then, regardless of which branch is taken, finish up as usual by checking for the deployment sentinel, and trigger a new run_next_capture if not present.

Why the big diff??

settings_common.py has gotten messy over time, and related settings were scattered all around. I wanted to add this feature flag, and kept waffling over where it belonged in the file... and I decided to finally just tidy up a bit.

So the first commit is literally just me cutting and pasting settings around the file (sorry git-blame).

While doing that I spotted a couple unused ones, that I failed to delete in the PR that removed the code that used them.

Finally: the strategy I used to test and see if this broke anything locally, which is to say, running the invoke test command, actually uncovered two syntax problems caused by changes made in the past few months. (CI runs pytest directly and so didn't catch these... and evidently I've gotten in the habit of invoking pytest directly locally too.)

So! A little messy, but hopefully acceptably so.

Why this strategy?

I wanted to keep the diff in celery_tasks.py as small as possible: I did NOT want to wrap the whole thing in a big if/else and thereby have to re-indent hundreds of lines of code.

I also wanted to make this flexible enough to allow us to easily experiment with/demo the other integration possibilities, even as we move forward with the Scoop REST API. By adding additional options to the flag (say, scoop-in-docker or scoop-api-v2 or whatever), we can neatly insert more alternatives as desired.