Block Transformers: distinguish between READ and WRITE versions
TNL-6522
This commit is contained in:
@@ -730,9 +730,9 @@ class BlockStructureBlockData(BlockStructure):
|
||||
Adds the given transformer to the block structure by recording
|
||||
its current version number.
|
||||
"""
|
||||
if transformer.VERSION == 0:
|
||||
raise TransformerException('VERSION attribute is not set on transformer {0}.', transformer.name())
|
||||
self.set_transformer_data(transformer, TRANSFORMER_VERSION_KEY, transformer.VERSION)
|
||||
if transformer.READ_VERSION == 0 or transformer.WRITE_VERSION == 0:
|
||||
raise TransformerException('Version attributes are not set on transformer {0}.', transformer.name())
|
||||
self.set_transformer_data(transformer, TRANSFORMER_VERSION_KEY, transformer.WRITE_VERSION)
|
||||
|
||||
def _get_or_create_block(self, usage_key):
|
||||
"""
|
||||
|
||||
@@ -3,15 +3,37 @@ Application-specific exceptions raised by the block structure framework.
|
||||
"""
|
||||
|
||||
|
||||
class TransformerException(Exception):
|
||||
class BlockStructureException(Exception):
|
||||
"""
|
||||
Base class for all Block Structure framework exceptions.
|
||||
"""
|
||||
pass
|
||||
|
||||
|
||||
class TransformerException(BlockStructureException):
|
||||
"""
|
||||
Exception class for Transformer related errors.
|
||||
"""
|
||||
pass
|
||||
|
||||
|
||||
class UsageKeyNotInBlockStructure(Exception):
|
||||
class UsageKeyNotInBlockStructure(BlockStructureException):
|
||||
"""
|
||||
Exception for when a usage key is not found within a block structure.
|
||||
"""
|
||||
pass
|
||||
|
||||
|
||||
class TransformerDataIncompatible(BlockStructureException):
|
||||
"""
|
||||
Exception for when the version of a Transformer's data is not
|
||||
compatible with the current version of the Transformer.
|
||||
"""
|
||||
pass
|
||||
|
||||
|
||||
class BlockStructureNotFound(BlockStructureException):
|
||||
"""
|
||||
Exception for when a Block Structure is not found.
|
||||
"""
|
||||
pass
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
Module for factory class for BlockStructure objects.
|
||||
"""
|
||||
from .block_structure import BlockStructureModulestoreData, BlockStructureBlockData
|
||||
from .exceptions import BlockStructureNotFound
|
||||
|
||||
|
||||
class BlockStructureFactory(object):
|
||||
@@ -77,11 +78,16 @@ class BlockStructureFactory(object):
|
||||
|
||||
Returns:
|
||||
BlockStructure - The deserialized block structure starting
|
||||
at root_block_usage_key, if found in the cache.
|
||||
at root_block_usage_key, if found in the cache.
|
||||
|
||||
NoneType - If the root_block_usage_key is not found in the cache.
|
||||
Raises:
|
||||
BlockStructureNotFound - If the root_block_usage_key is not found
|
||||
in the cache.
|
||||
"""
|
||||
return block_structure_cache.get(root_block_usage_key)
|
||||
block_structure = block_structure_cache.get(root_block_usage_key)
|
||||
if block_structure is None:
|
||||
raise BlockStructureNotFound('Block structure for {} not found in the cache.'.format(root_block_usage_key))
|
||||
return block_structure
|
||||
|
||||
@classmethod
|
||||
def create_new(cls, root_block_usage_key, block_relations, transformer_data, block_data_map):
|
||||
|
||||
@@ -6,7 +6,7 @@ from contextlib import contextmanager
|
||||
|
||||
from .cache import BlockStructureCache
|
||||
from .factory import BlockStructureFactory
|
||||
from .exceptions import UsageKeyNotInBlockStructure
|
||||
from .exceptions import UsageKeyNotInBlockStructure, TransformerDataIncompatible, BlockStructureNotFound
|
||||
from .transformers import BlockStructureTransformers
|
||||
|
||||
|
||||
@@ -89,13 +89,16 @@ class BlockStructureManager(object):
|
||||
starting at root_block_usage_key, with collected data
|
||||
from each registered transformer.
|
||||
"""
|
||||
block_structure = BlockStructureFactory.create_from_cache(
|
||||
self.root_block_usage_key,
|
||||
self.block_structure_cache
|
||||
)
|
||||
cache_miss = block_structure is None
|
||||
if cache_miss or BlockStructureTransformers.is_collected_outdated(block_structure):
|
||||
try:
|
||||
block_structure = BlockStructureFactory.create_from_cache(
|
||||
self.root_block_usage_key,
|
||||
self.block_structure_cache
|
||||
)
|
||||
BlockStructureTransformers.verify_versions(block_structure)
|
||||
|
||||
except (BlockStructureNotFound, TransformerDataIncompatible):
|
||||
block_structure = self.update_collected()
|
||||
|
||||
return block_structure
|
||||
|
||||
def update_collected(self):
|
||||
|
||||
@@ -136,7 +136,8 @@ class MockTransformer(BlockStructureTransformer):
|
||||
"""
|
||||
A mock BlockStructureTransformer class.
|
||||
"""
|
||||
VERSION = 1
|
||||
WRITE_VERSION = 1
|
||||
READ_VERSION = 1
|
||||
|
||||
@classmethod
|
||||
def name(cls):
|
||||
@@ -151,7 +152,8 @@ class MockFilteringTransformer(FilteringTransformerMixin, BlockStructureTransfor
|
||||
"""
|
||||
A mock FilteringTransformerMixin class.
|
||||
"""
|
||||
VERSION = 1
|
||||
WRITE_VERSION = 1
|
||||
READ_VERSION = 1
|
||||
|
||||
@classmethod
|
||||
def name(cls):
|
||||
|
||||
@@ -56,11 +56,12 @@ class TestBlockStructureData(TestCase, ChildrenMapTestMixin):
|
||||
"""
|
||||
Test transformer with default version number (0).
|
||||
"""
|
||||
VERSION = 0
|
||||
WRITE_VERSION = 0
|
||||
READ_VERSION = 0
|
||||
|
||||
block_structure = BlockStructureModulestoreData(root_block_usage_key=0)
|
||||
|
||||
with self.assertRaisesRegexp(TransformerException, "VERSION attribute is not set"):
|
||||
with self.assertRaisesRegexp(TransformerException, "Version attributes are not set"):
|
||||
block_structure._add_transformer(TestNonVersionedTransformer())
|
||||
|
||||
def test_transformer_data(self):
|
||||
@@ -103,7 +104,7 @@ class TestBlockStructureData(TestCase, ChildrenMapTestMixin):
|
||||
for t_info in transformers_info:
|
||||
self.assertEquals(
|
||||
block_structure._get_transformer_data_version(t_info.transformer),
|
||||
MockTransformer.VERSION
|
||||
MockTransformer.WRITE_VERSION
|
||||
)
|
||||
for key, val in t_info.structure_wide_data:
|
||||
self.assertEquals(
|
||||
|
||||
@@ -6,6 +6,7 @@ from unittest import TestCase
|
||||
from xmodule.modulestore.exceptions import ItemNotFoundError
|
||||
|
||||
from ..cache import BlockStructureCache
|
||||
from ..exceptions import BlockStructureNotFound
|
||||
from ..factory import BlockStructureFactory
|
||||
from .helpers import (
|
||||
MockCache, MockModulestoreFactory, ChildrenMapTestMixin
|
||||
@@ -43,17 +44,15 @@ class TestBlockStructureFactory(TestCase, ChildrenMapTestMixin):
|
||||
block_structure.root_block_usage_key,
|
||||
cache,
|
||||
)
|
||||
self.assertIsNotNone(from_cache_block_structure)
|
||||
self.assert_block_structure(from_cache_block_structure, self.children_map)
|
||||
|
||||
def test_from_cache_none(self):
|
||||
cache = BlockStructureCache(MockCache())
|
||||
self.assertIsNone(
|
||||
with self.assertRaises(BlockStructureNotFound):
|
||||
BlockStructureFactory.create_from_cache(
|
||||
root_block_usage_key=0,
|
||||
block_structure_cache=cache,
|
||||
)
|
||||
)
|
||||
|
||||
def test_new(self):
|
||||
block_structure = BlockStructureFactory.create_from_modulestore(
|
||||
|
||||
@@ -167,15 +167,24 @@ class TestBlockStructureManager(TestCase, ChildrenMapTestMixin):
|
||||
self.collect_and_verify(expect_modulestore_called=False, expect_cache_updated=False)
|
||||
self.assertEquals(TestTransformer1.collect_call_count, 1)
|
||||
|
||||
def test_get_collected_outdated_data(self):
|
||||
def test_get_collected_transformer_version(self):
|
||||
self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True)
|
||||
TestTransformer1.VERSION += 1 # transformer code requires new schema version
|
||||
self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True)
|
||||
TestTransformer1.VERSION -= 1 # old transformer code works with new schema version
|
||||
|
||||
# transformer code writes new schema version; data not re-collected
|
||||
TestTransformer1.WRITE_VERSION += 1
|
||||
self.collect_and_verify(expect_modulestore_called=False, expect_cache_updated=False)
|
||||
|
||||
# transformer code requires new schema version; data re-collected
|
||||
TestTransformer1.READ_VERSION += 1
|
||||
self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True)
|
||||
|
||||
# old transformer code can read new schema version; data not re-collected
|
||||
TestTransformer1.READ_VERSION -= 1
|
||||
self.collect_and_verify(expect_modulestore_called=False, expect_cache_updated=False)
|
||||
|
||||
self.assertEquals(TestTransformer1.collect_call_count, 2)
|
||||
|
||||
def test_get_collected_version_update(self):
|
||||
def test_get_collected_structure_version(self):
|
||||
self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True)
|
||||
BlockStructureBlockData.VERSION += 1
|
||||
self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True)
|
||||
|
||||
@@ -6,7 +6,7 @@ from nose.plugins.attrib import attr
|
||||
from unittest import TestCase
|
||||
|
||||
from ..block_structure import BlockStructureModulestoreData
|
||||
from ..exceptions import TransformerException
|
||||
from ..exceptions import TransformerException, TransformerDataIncompatible
|
||||
from ..transformers import BlockStructureTransformers
|
||||
from .helpers import (
|
||||
ChildrenMapTestMixin, MockTransformer, MockFilteringTransformer, mock_registered_transformers
|
||||
@@ -71,13 +71,14 @@ class TestBlockStructureTransformers(ChildrenMapTestMixin, TestCase):
|
||||
self.transformers.transform(block_structure=MagicMock())
|
||||
self.assertTrue(mock_transform_call.called)
|
||||
|
||||
def test_is_collected_outdated(self):
|
||||
def test_verify_versions(self):
|
||||
block_structure = self.create_block_structure(
|
||||
self.SIMPLE_CHILDREN_MAP,
|
||||
BlockStructureModulestoreData
|
||||
)
|
||||
|
||||
with mock_registered_transformers(self.registered_transformers):
|
||||
self.assertTrue(self.transformers.is_collected_outdated(block_structure))
|
||||
with self.assertRaises(TransformerDataIncompatible):
|
||||
self.transformers.verify_versions(block_structure)
|
||||
self.transformers.collect(block_structure)
|
||||
self.assertFalse(self.transformers.is_collected_outdated(block_structure))
|
||||
self.assertTrue(self.transformers.verify_versions(block_structure))
|
||||
|
||||
@@ -10,23 +10,50 @@ class BlockStructureTransformer(object):
|
||||
Abstract base class for all block structure transformers.
|
||||
"""
|
||||
|
||||
# All Transformers are expected to maintain a VERSION class
|
||||
# attribute. While the value for the base class is set to 0,
|
||||
# the value for each concrete transformer should be 1 or higher.
|
||||
# All Transformers are expected to maintain version-related class
|
||||
# attributes. While the values for the base class is set to 0,
|
||||
# the values for each concrete transformer should be 1 or higher.
|
||||
#
|
||||
# A transformer's version attribute is used by the block_structure
|
||||
# A transformer's version attributes are used by the block_structure
|
||||
# framework in order to determine whether any collected data for a
|
||||
# transformer is outdated. When a transformer's data is collected
|
||||
# and cached, it's version number at the time of collection is
|
||||
# stored along with the data. That version number is then checked
|
||||
# at the time of accessing the collected data (during the transform
|
||||
# phase).
|
||||
# transformer is outdated because of a data schema change by the
|
||||
# transformer.
|
||||
#
|
||||
# The version number of a Transformer should be incremented each
|
||||
# time the implementation of its collect method is updated such that
|
||||
# its collected data is changed.
|
||||
# The WRITE_VERSION number is stored along with the transformer's
|
||||
# data when it is collected and cached (during the collect phase).
|
||||
# The READ_VERSION number is then verified to be less than or equal
|
||||
# to the version associated with the collected data when the
|
||||
# collected data is accessed (during the transform phase).
|
||||
#
|
||||
VERSION = 0
|
||||
# We distinguish between WRITE_VERSION and READ_VERSION numbers in
|
||||
# order to:
|
||||
# 1. support blue-green deployments where new and previous versions
|
||||
# of the code base are simultaneously executing on different
|
||||
# workers for a period of time.
|
||||
#
|
||||
# A 2-phase deployment is used to stagger read and write changes.
|
||||
#
|
||||
# 2. scale for large deployments where it is costly to recompute
|
||||
# block structures for all courses when a transformer's collected
|
||||
# data schema changes.
|
||||
#
|
||||
# A background management command is run to prime the new data.
|
||||
#
|
||||
# See the following document for further information:
|
||||
# https://openedx.atlassian.net/wiki/display/MA/Block+Structure+Cache+Invalidation+Proposal
|
||||
#
|
||||
# The WRITE_VERSION number of a Transformer should be incremented
|
||||
# when it's collect implementation is additively changed. Backward
|
||||
# compatibility should be maintained with previous READ_VERSIONs
|
||||
# until all readers are updated.
|
||||
#
|
||||
# The READ_VERSION number of a Transformer should be incremented
|
||||
# when its transform implementation is updated to make use of the
|
||||
# newly collected data - and released only after all collected
|
||||
# block structures are updated with the new WRITE_VERSION.
|
||||
#
|
||||
WRITE_VERSION = 0
|
||||
READ_VERSION = 0
|
||||
|
||||
@classmethod
|
||||
def name(cls):
|
||||
|
||||
@@ -4,7 +4,7 @@ Module for a collection of BlockStructureTransformers.
|
||||
import functools
|
||||
from logging import getLogger
|
||||
|
||||
from .exceptions import TransformerException
|
||||
from .exceptions import TransformerException, TransformerDataIncompatible
|
||||
from .transformer import FilteringTransformerMixin
|
||||
from .transformer_registry import TransformerRegistry
|
||||
|
||||
@@ -83,23 +83,27 @@ class BlockStructureTransformers(object):
|
||||
block_structure._collect_requested_xblock_fields() # pylint: disable=protected-access
|
||||
|
||||
@classmethod
|
||||
def is_collected_outdated(cls, block_structure):
|
||||
def verify_versions(cls, block_structure):
|
||||
"""
|
||||
Returns whether the collected data in the block structure is outdated.
|
||||
Returns whether the collected data in the block structure is
|
||||
incompatible with the current version of the registered Transformers.
|
||||
|
||||
Raises:
|
||||
TransformerDataIncompatible with information about all outdated
|
||||
Transformers.
|
||||
"""
|
||||
outdated_transformers = []
|
||||
for transformer in TransformerRegistry.get_registered_transformers():
|
||||
version_in_block_structure = block_structure._get_transformer_data_version(transformer) # pylint: disable=protected-access
|
||||
if transformer.VERSION > version_in_block_structure:
|
||||
if transformer.READ_VERSION > version_in_block_structure:
|
||||
outdated_transformers.append(transformer)
|
||||
|
||||
if outdated_transformers:
|
||||
logger.info(
|
||||
raise TransformerDataIncompatible(
|
||||
"Collected Block Structure data for the following transformers is outdated: '%s'.",
|
||||
[(transformer.name(), transformer.VERSION) for transformer in outdated_transformers],
|
||||
[(transformer.name(), transformer.READ_VERSION) for transformer in outdated_transformers],
|
||||
)
|
||||
|
||||
return bool(outdated_transformers)
|
||||
return True
|
||||
|
||||
def transform(self, block_structure):
|
||||
"""
|
||||
|
||||
Reference in New Issue
Block a user