Refactor several poorly named fields on Version

[Mr0grog]

This is a first pass at the remaining items in #776. The Version model is pretty central to the whole system, so this is kind of a big and very tedious change. :\

This makes the following changes to Version:

• Rename capture_url → url
• Rename uri → body_url
• Rename version_hash → body_hash
• Maybe rename content_length → body_length
• Add headers (a.k.a. move source_metadata.headers → headers)

Note that the content_length change hasn’t yet been implemented here. I’m a little on the fence. The current name is clear, and references a well known HTTP header. The proposed new name (body_length) is still clear and is more concise, but departs from well-known convention.

Remaining work to do here:

• Migration
• Docs
• Main code changes
• Add data migration to fill in headers from existing source_metadata.headers data
• Add tests to ensure imports based on the old format don't break
• Decide on content_length → body_length
• Update seed files
• Update downstream systems (web-monitoring-processing, web-monitoring-task-sheets, web-monitoring-ui, web-monitoring-changed-terms-analysis, web-monitoring-versionista-scraper — not sure these last two really matter since they are not actively used anymore)

Fixes #776.

[Mr0grog]

OK, I think I’m not going to change content_length. It’s pretty clear as-is, and the shortness/consistency of body_length is a really minor benefit vs. the issues with refactoring the API here.

[Mr0grog]

Migrated the seed file with the following quick jq commands:

$ cat db/seed_import.json | jq -c '. + {body_hash: .version_hash, body_url: .uri} | del(.version_hash) | del(.uri)' > db/seed_import.new.json
$ mv db/seed_import.new.json db/seed_import.json

(No need to update capture_url because it’s not used here.)

[Mr0grog]

Downstream update PRs:

• Handle both old and new field names for versions web-monitoring-ui#749
• Handle both old and new field names for versions web-monitoring-processing#727
• Handle both old and new field names for versions web-monitoring-task-sheets#10

Not going to worry about -versionista-scraper and -changed-terms-analysis since they are no longer in active use.

[Mr0grog]

OK, this should be good to go pending updates to downstream consumers.

Going to let this bake for a while and come back with fresh eyes and and a careful review tomorrow or later in the week.

[danielballan]

Renames done in pairs like this that effectively change the meaning of term can create hard-to-find bugs:

• Rename capture_url → url
• Rename uri → body_url

Are we sure we like capture_url -> url better enough to risk it? Alternative would be deprecating url entirely so that anywhere it's left over we know unambiguously that it's wrong and what it should be changed to.

[Mr0grog]

Are we sure we like capture_url -> url better enough to risk it?

Ha! These are actually the ones I feel most confident about, even though you are right that they are the most technically risky. I feel confident about these for two reasons:

• Technical issues are unlikely, because url and uri are still different, so any existing code won’t suddenly be doing the wrong thing.
• This is resolving longstanding confusion and questions I’ve gotten based on the names; we’re aligning them with what people intuitively expect them to mean, which is the opposite of what they currently are:
  • capture_url is ambiguous about whether it represents the URL that was captured or the URL where the capture resides (another way to clarify this might be capture_url → captured_url). But I think people mostly expect url or uri to be the URL that was captured, which leads me to…
  • uri → body_url is similarly very clarifying because people often first expect that uri is representing the URL/URI that was captured, rather than where the capture is stored.

[danielballan]

Those justifications are convincing. I had actually missed the uri / url distinction in my first read. Now it's clear that's a nonissue.

[Mr0grog]

Going to merge this in an hour or so after weekly sheets are done building.

[Mr0grog]

Deployed to staging and and ran the data migration there. Took 79 minutes, but otherwise worked great. Deploying to production now. :)