Page MenuHomePhabricator

Update Wikibase entity saving logic to use PropertyTermStore
Closed, ResolvedPublic

Description

PropertyTermStore should be used when migration is switched to write new mode.

Event Timeline

Currently, we have this in WikibaseRepo:

	private function newEntityTermStoreWriter(): EntityTermStoreWriter {
		$termStoreMigrationStage = $this->settings->getSetting( 'tmpTermStoreMigrationStage' );

		if ( $termStoreMigrationStage === MIGRATION_WRITE_BOTH ) {
			return new MultiTermStoreWriter(
				$this->getOldEntityTermStoreWriter(),
				$this->getNewEntityTermStoreWriter()
			);
		}

		if ( $termStoreMigrationStage === MIGRATION_WRITE_NEW ) {
			return $this->getNewEntityTermStoreWriter();
		}

		return $this->getOldEntityTermStoreWriter();
	}

	private function getOldEntityTermStoreWriter(): EntityTermStoreWriter {
		return $this->getStore()->getTermIndex();
	}

	private function getNewEntityTermStoreWriter(): EntityTermStoreWriter {
		return new DelegatingEntityTermStoreWriter(
			$this->getPropertyTermStore(),
			$this->getItemTermStore()
		);
	}

I can only assume that this predates the current migration plan, because it doesn’t really fit it very well: it assumes that we’ll do a migration across all entities at the same time, whereas we want to do properties first, and then items only up to a certain numeric ID.

Change 513118 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Add TermIndexPropertyTermStore adapter

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

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

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

Change 513158 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Wire up DatabasePropertyTermStore in WikibaseRepo

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

Currently, we have this in WikibaseRepo:

	private function newEntityTermStoreWriter(): EntityTermStoreWriter {
		$termStoreMigrationStage = $this->settings->getSetting( 'tmpTermStoreMigrationStage' );
		if ( $termStoreMigrationStage === MIGRATION_WRITE_BOTH ) {
			return new MultiTermStoreWriter(
				$this->getOldEntityTermStoreWriter(),
				$this->getNewEntityTermStoreWriter()
			);
		}
		if ( $termStoreMigrationStage === MIGRATION_WRITE_NEW ) {
			return $this->getNewEntityTermStoreWriter();
		}
		return $this->getOldEntityTermStoreWriter();
	}
	private function getOldEntityTermStoreWriter(): EntityTermStoreWriter {
		return $this->getStore()->getTermIndex();
	}
	private function getNewEntityTermStoreWriter(): EntityTermStoreWriter {
		return new DelegatingEntityTermStoreWriter(
			$this->getPropertyTermStore(),
			$this->getItemTermStore()
		);
	}

I can only assume that this predates the current migration plan, because it doesn’t really fit it very well: it assumes that we’ll do a migration across all entities at the same time, whereas we want to do properties first, and then items only up to a certain numeric ID.

Yeap that was introduced too early, without much attention to the actual migration plan. (I do remember rejecting that change, but maybe I'm confusing it with something else).

Change 513118 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Add TermIndexPropertyTermStore adapter

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

Change 513144 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Add MultiPropertyTermStore

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

Change 513158 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Wire up DatabasePropertyTermStore in WikibaseRepo

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

Lucas_Werkmeister_WMDE closed this task as Resolved.Jun 3 2019, 4:23 PM

Change 514078 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Add TermIndexItemTermStore adapter

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

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

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

Change 514078 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Add TermIndexItemTermStore adapter

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

Change 514079 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Add MultiItemTermStore

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