Page MenuHomePhabricator

[2hrs] PageImages code should have documentation warning developers about caching implications
Closed, ResolvedPublic

Description

There is no inline documentation or note in the README in the PageImages extension about how PageImages are populated and what developers should be wary of when working with this codebase. Let's add some to protect against incidents such as https://wikitech.wikimedia.org/wiki/Incident_documentation/20161202-20161201-PageImages

Event Timeline

jhobs triaged this task as Medium priority.Dec 7 2016, 5:28 PM
jhobs set the point value for this task to 0.5.
jhobs moved this task from Incoming to Triaged but Future on the Web-Team-Backlog board.
ovasileva subscribed.

@bmansurov - did we identify any next steps on this?

@ovasileva could you clarify what you mean? I think this task is self-containing.

Jdlrobson renamed this task from PageImages code should have documentation warning developers about caching implications to [2hrs] PageImages code should have documentation warning developers about caching implications.Jan 18 2017, 6:33 PM
Jdlrobson removed the point value for this task.

Change 333805 had a related patch set uploaded (by Jdlrobson):
Document page property names

https://gerrit.wikimedia.org/r/333805

I've +2'ed the above patch. Any plans to update the README file as well, @Jdlrobson?

Change 333805 merged by jenkins-bot:
Document page property names

https://gerrit.wikimedia.org/r/333805

It seemed an inline comment was most appropriate here.

We should document the config variables in a readme and how they behave but I don't see how this would help protect us against license issues like that in the incident since there is no config switch that impacts it.

We should document the config variables in a readme and how they behave but I don't see how this would help protect us against license issues like that in the incident since there is no config switch that impacts it.

This task is about documenting caching implications gotchas around the licensing-related config variables and not necessarily licensing issues themselves. A note in the README seems appropriate.

There are no licensing related config; that's my point. The inline comment is most appropriate in my opinion then a out of context note in a read me... Which is done.

Mibad. I'd assumed that the free-by-default behaviour was flagged…

^ Thinking about my assumptions: am I correct in assuming that we announced the new default on a mailing list?

I couldn't find an announcement, though this is likely not finding the right Google incantation. The most recent changes have been discussed a number of times on reading-wmf and well documented on mediawiki.org.

I think this task can be considered done since the new behaviour is the default and it's not flagged.

I suggest we discuss the new default as part of sign off for T152216.

bmansurov removed bmansurov as the assignee of this task.