Page MenuHomePhabricator
Create Task
Maniphest T189593

Replace CachingResultsBuilder with CachingResultsSource, (de)serializing CheckResults
Closed, ResolvedPublic
Actions

Description

We don’t need any hybrid phase for T185709: Cache CheckResult serializations per-entity in ObjectCache, so we can probably just exchange the CachingResultsBuilder implementation for one that reads and writes CheckResult serializations instead (with a separate cache key format… v2.2 I guess).

The transition is complicated enough that it’s split up across several patches. Essentially, were replacing ResultsBuilder with ResultsSource plus CheckResultsRenderer.

Patch-For-Review:

Details

SubjectRepoBranchLines +/-
Update help message for status parametermediawiki/extensions/WikibaseQualityConstraintsmaster+1 -1
Remove ResultsBuilder and implementationsmediawiki/extensions/WikibaseQualityConstraintsmaster+0 -1 K
Use ResultsSource instead of ResultsBuildermediawiki/extensions/WikibaseQualityConstraintsmaster+69 -107
Add CachingResultsSourcemediawiki/extensions/WikibaseQualityConstraintsmaster+1 K -0
Add CheckResultsRenderermediawiki/extensions/WikibaseQualityConstraintsmaster+365 -0
Make ResultsCache format version configurablemediawiki/extensions/WikibaseQualityConstraintsmaster+63 -39
Add ResultsSource and CheckingResultsSourcemediawiki/extensions/WikibaseQualityConstraintsmaster+443 -0
Add CachedCheckResultsmediawiki/extensions/WikibaseQualityConstraintsmaster+87 -0
Customize query in gerrit

Related Objects
Search...

Event Timeline

Lucas_Werkmeister_WMDE triaged this task as Medium priority.Mar 13 2018, 3:13 PM
Lucas_Werkmeister_WMDE created this task.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 13 2018, 3:13 PM
Lucas_Werkmeister_WMDE mentioned this in T185709: Cache CheckResult serializations per-entity in ObjectCache.Mar 13 2018, 3:25 PM
Lydia_Pintscher moved this task from incoming to in progress on the Wikidata board.Mar 13 2018, 4:38 PM
Lucas_Werkmeister_WMDE added a comment.Mar 19 2018, 3:22 PM

Hm, we don’t need a hybrid phase, but this is still a big change – and it pretty much invalidates the current interface of CachedCheckConstraintsResponse and ResultsBuilder, so perhaps it would be better to build up a new interface with new implementations in several commits, and then remove the old implementations afterwards, so it’s not in one huge, monolithic commit.

Lucas_Werkmeister_WMDE added a comment.Mar 19 2018, 4:08 PM

So what do we have instead?

  • CachedCheckResults: wrapper around CheckResult[] + Metadata. (Note: since the results may be filtered, the overall metadata may be more than the merge of the individual CheckResult metadatas!)
  • something which takes the same parameters as ResultsBuilder::getResults (entity IDs, claim IDs, constraint IDs, statuses), but returns CachedCheckResults instead of a CachedCheckConstraintsResponse. I don’t have a good name for this yet… it does even less than ResultsBuilder did (at least the implementation that doesn’t do caching). A ResultsGatherer, perhaps? But CachingResultsGatherer doesn’t really make too much sense.
Lucas_Werkmeister_WMDE claimed this task.Mar 19 2018, 4:33 PM
Lucas_Werkmeister_WMDE moved this task from Tasks to In Progress on the Wikidata-Ministry-Of-Magic board.
Lucas_Werkmeister_WMDE added a comment.Mar 19 2018, 5:08 PM

Perhaps a CheckResultsSource? But that doesn’t really yield any good names for the implementation (a CheckingCheckResultsSource?).

Lucas_Werkmeister_WMDE added a comment.Mar 19 2018, 5:14 PM

I suppose it could be just a ResultsSource. Very generic, but matches ResultsBuilder. CheckingResultsSource and CachingResultsSource don’t sound too bad to me.

Lucas_Werkmeister_WMDE added a comment.Mar 19 2018, 5:37 PM

Oh, and we need a place where we actually do the conversion of CheckResults to arrays and store them all in the giant result structure, since ResultsSource doesn’t do that anymore. (This is also the step where ViolationMessages are rendered.) Is that a CheckResultsRenderer?

gerritbot subscribed.Mar 20 2018, 1:58 PM

Change 420706 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachedCheckResults

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

gerritbot added a project: Patch-For-Review.Mar 20 2018, 1:58 PM

Change 420707 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add ResultsSource and CheckingResultsSource

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

gerritbot added a comment.Mar 20 2018, 1:58 PM

Change 420708 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CheckResultsRenderer

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

Lucas_Werkmeister_WMDE renamed this task from (De)serialize CheckResults in CachingResultsBuilder to Replace CachingResultsBuilder with CachingResultsSource, (de)serializing CheckResults.Mar 20 2018, 2:03 PM
Lucas_Werkmeister_WMDE updated the task description. (Show Details)
Lucas_Werkmeister_WMDE moved this task from In Progress to Needs Review on the Wikidata-Ministry-Of-Magic board.
gerritbot added a comment.Mar 20 2018, 2:35 PM

Change 420719 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Make ResultsCache format version configurable

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

Lucas_Werkmeister_WMDE updated the task description. (Show Details)Mar 20 2018, 2:36 PM
Lucas_Werkmeister_WMDE updated the task description. (Show Details)Mar 20 2018, 7:33 PM
Lucas_Werkmeister_WMDE added a comment.Mar 20 2018, 7:55 PM

make the "v2.1" part of the cache key in ResultsCache configurable – we need "v2.1" for CachingResultsBuilder but "v2.2" for CachingResultsSource

Um. Except that the whole point of this entire exercise is to reuse the same cache for all languages, which means a change to the cache key anyways… 🤦

(It still makes sense to bump the key to v2.2, but it’s pretty stupid how long it took me to realize this ^^ )

gerritbot added a comment.Mar 20 2018, 8:03 PM

Change 420834 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachingResultsSource

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

gerritbot added a comment.Mar 20 2018, 8:03 PM

Change 420835 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] DNM: Use ResultsSource instead of ResultsBuilder

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

Lucas_Werkmeister_WMDE updated the task description. (Show Details)Mar 20 2018, 8:03 PM
gerritbot added a comment.Mar 21 2018, 11:33 AM

Change 420706 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachedCheckResults

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

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-03-27 (1.31.0-wmf.27)).Mar 21 2018, 12:00 PM
Lucas_Werkmeister_WMDE updated the task description. (Show Details)Mar 21 2018, 12:45 PM
gerritbot added a comment.Mar 22 2018, 10:08 AM

Change 420707 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add ResultsSource and CheckingResultsSource

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

gerritbot added a comment.Mar 22 2018, 10:16 AM

Change 420708 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CheckResultsRenderer

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

gerritbot added a comment.Mar 22 2018, 10:16 AM

Change 420719 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Make ResultsCache format version configurable

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

Ladsgroup subscribed.Mar 22 2018, 12:44 PM

Is everything here merged?

Lucas_Werkmeister_WMDE updated the task description. (Show Details)Mar 22 2018, 1:04 PM
Ladsgroup moved this task from Needs Review to In Progress on the Wikidata-Ministry-Of-Magic board.Mar 23 2018, 12:23 PM
Lucas_Werkmeister_WMDE mentioned this in T190672: Cache check results from Special:ConstraintReport checks.Mar 26 2018, 11:09 AM
Lucas_Werkmeister_WMDE moved this task from In Progress to Needs Review on the Wikidata-Ministry-Of-Magic board.Mar 26 2018, 11:50 AM
gerritbot added a comment.Mar 26 2018, 12:03 PM

Change 421873 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Update help message for status parameter

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

Lucas_Werkmeister_WMDE updated the task description. (Show Details)Mar 26 2018, 12:03 PM
gerritbot added a comment.Mar 26 2018, 12:10 PM

Change 421875 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Remove ResultsBuilder and implementations

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

Lucas_Werkmeister_WMDE updated the task description. (Show Details)Mar 26 2018, 12:10 PM
gerritbot added a comment.Mar 26 2018, 12:35 PM

Change 420834 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add CachingResultsSource

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

gerritbot added a comment.Mar 26 2018, 12:53 PM

Change 420835 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Use ResultsSource instead of ResultsBuilder

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

gerritbot added a comment.Mar 26 2018, 12:53 PM

Change 421873 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Update help message for status parameter

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

gerritbot added a comment.Mar 26 2018, 12:53 PM

Change 421875 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Remove ResultsBuilder and implementations

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

Lucas_Werkmeister_WMDE closed this task as Resolved.Mar 26 2018, 12:54 PM
Lucas_Werkmeister_WMDE updated the task description. (Show Details)
Lucas_Werkmeister_WMDE moved this task from Needs Review to Done on the Wikidata-Ministry-Of-Magic board.
Lucas_Werkmeister_WMDE mentioned this in T189842: Undefined index error in CachingResultsBuilder when checking constraints on empty entity.Mar 26 2018, 2:39 PM
Lucas_Werkmeister_WMDE mentioned this in T183257: Introduce ConstraintReport class.Apr 6 2018, 3:05 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL