From 71cce9bb738ac6cfd4098c2f88fb39ea6103883c Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Sat, 11 Feb 2017 11:47:51 -0500 Subject: [PATCH] Block Transformers: distinguish between READ and WRITE versions TNL-6522 --- .../blocks/transformers/block_counts.py | 3 +- .../blocks/transformers/block_depth.py | 3 +- .../blocks/transformers/blocks_api.py | 3 +- .../blocks/transformers/milestones.py | 3 +- .../blocks/transformers/navigation.py | 3 +- .../blocks/transformers/student_view.py | 3 +- .../transformers/hidden_content.py | 3 +- .../transformers/library_content.py | 3 +- .../course_blocks/transformers/split_test.py | 3 +- .../course_blocks/transformers/start_date.py | 3 +- .../transformers/user_partitions.py | 3 +- .../course_blocks/transformers/visibility.py | 3 +- lms/djangoapps/grades/tests/test_tasks.py | 3 +- lms/djangoapps/grades/transformer.py | 7 +-- .../lib/block_structure/block_structure.py | 6 +-- .../core/lib/block_structure/exceptions.py | 26 ++++++++- openedx/core/lib/block_structure/factory.py | 12 +++-- openedx/core/lib/block_structure/manager.py | 17 +++--- .../core/lib/block_structure/tests/helpers.py | 6 ++- .../tests/test_block_structure.py | 7 +-- .../lib/block_structure/tests/test_factory.py | 5 +- .../lib/block_structure/tests/test_manager.py | 19 +++++-- .../tests/test_transformers.py | 9 ++-- .../core/lib/block_structure/transformer.py | 53 ++++++++++++++----- .../core/lib/block_structure/transformers.py | 20 ++++--- 25 files changed, 155 insertions(+), 71 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/transformers/block_counts.py b/lms/djangoapps/course_api/blocks/transformers/block_counts.py index 2bab0f912e..ea38c3ed13 100644 --- a/lms/djangoapps/course_api/blocks/transformers/block_counts.py +++ b/lms/djangoapps/course_api/blocks/transformers/block_counts.py @@ -8,7 +8,8 @@ class BlockCountsTransformer(BlockStructureTransformer): """ Keep a count of descendant blocks of the requested types """ - VERSION = 1 + WRITE_VERSION = 1 + READ_VERSION = 1 BLOCK_COUNTS = 'block_counts' def __init__(self, block_types_to_count): diff --git a/lms/djangoapps/course_api/blocks/transformers/block_depth.py b/lms/djangoapps/course_api/blocks/transformers/block_depth.py index 97ca27a59a..889bf1f987 100644 --- a/lms/djangoapps/course_api/blocks/transformers/block_depth.py +++ b/lms/djangoapps/course_api/blocks/transformers/block_depth.py @@ -9,7 +9,8 @@ class BlockDepthTransformer(BlockStructureTransformer): Keep track of the depth of each block within the block structure. In case of multiple paths to a given node (in a DAG), use the shallowest depth. """ - VERSION = 1 + WRITE_VERSION = 1 + READ_VERSION = 1 BLOCK_DEPTH = 'block_depth' def __init__(self, requested_depth=None): diff --git a/lms/djangoapps/course_api/blocks/transformers/blocks_api.py b/lms/djangoapps/course_api/blocks/transformers/blocks_api.py index f70b147898..1f99fc94da 100644 --- a/lms/djangoapps/course_api/blocks/transformers/blocks_api.py +++ b/lms/djangoapps/course_api/blocks/transformers/blocks_api.py @@ -22,7 +22,8 @@ class BlocksAPITransformer(BlockStructureTransformer): Note: BlockDepthTransformer must be executed before BlockNavigationTransformer. """ - VERSION = 1 + WRITE_VERSION = 1 + READ_VERSION = 1 STUDENT_VIEW_DATA = 'student_view_data' STUDENT_VIEW_MULTI_DEVICE = 'student_view_multi_device' diff --git a/lms/djangoapps/course_api/blocks/transformers/milestones.py b/lms/djangoapps/course_api/blocks/transformers/milestones.py index 33ce6a9680..e05c123228 100644 --- a/lms/djangoapps/course_api/blocks/transformers/milestones.py +++ b/lms/djangoapps/course_api/blocks/transformers/milestones.py @@ -13,7 +13,8 @@ class MilestonesTransformer(FilteringTransformerMixin, BlockStructureTransformer Excludes all special exams (timed, proctored, practice proctored) from the student view. Excludes all blocks with unfulfilled milestones from the student view. """ - VERSION = 1 + WRITE_VERSION = 1 + READ_VERSION = 1 @classmethod def name(cls): diff --git a/lms/djangoapps/course_api/blocks/transformers/navigation.py b/lms/djangoapps/course_api/blocks/transformers/navigation.py index 7fad43e8b6..643199adb5 100644 --- a/lms/djangoapps/course_api/blocks/transformers/navigation.py +++ b/lms/djangoapps/course_api/blocks/transformers/navigation.py @@ -20,7 +20,8 @@ class BlockNavigationTransformer(BlockStructureTransformer): Prerequisites: BlockDepthTransformer must be run before this in the transform phase. """ - VERSION = 1 + WRITE_VERSION = 1 + READ_VERSION = 1 BLOCK_NAVIGATION = 'block_nav' BLOCK_NAVIGATION_FOR_CHILDREN = 'children_block_nav' diff --git a/lms/djangoapps/course_api/blocks/transformers/student_view.py b/lms/djangoapps/course_api/blocks/transformers/student_view.py index d9da16f72e..aea7ce5ef4 100644 --- a/lms/djangoapps/course_api/blocks/transformers/student_view.py +++ b/lms/djangoapps/course_api/blocks/transformers/student_view.py @@ -8,7 +8,8 @@ class StudentViewTransformer(BlockStructureTransformer): """ Only show information that is appropriate for a learner """ - VERSION = 1 + WRITE_VERSION = 1 + READ_VERSION = 1 STUDENT_VIEW_DATA = 'student_view_data' STUDENT_VIEW_MULTI_DEVICE = 'student_view_multi_device' diff --git a/lms/djangoapps/course_blocks/transformers/hidden_content.py b/lms/djangoapps/course_blocks/transformers/hidden_content.py index 3f2e2275c2..559f2b1d35 100644 --- a/lms/djangoapps/course_blocks/transformers/hidden_content.py +++ b/lms/djangoapps/course_blocks/transformers/hidden_content.py @@ -25,7 +25,8 @@ class HiddenContentTransformer(FilteringTransformerMixin, BlockStructureTransfor Staff users are exempted from hidden content rules. """ - VERSION = 2 + WRITE_VERSION = 2 + READ_VERSION = 2 MERGED_DUE_DATE = 'merged_due_date' MERGED_HIDE_AFTER_DUE = 'merged_hide_after_due' diff --git a/lms/djangoapps/course_blocks/transformers/library_content.py b/lms/djangoapps/course_blocks/transformers/library_content.py index e2d4f9ac1f..943b0e1503 100644 --- a/lms/djangoapps/course_blocks/transformers/library_content.py +++ b/lms/djangoapps/course_blocks/transformers/library_content.py @@ -18,7 +18,8 @@ class ContentLibraryTransformer(FilteringTransformerMixin, BlockStructureTransfo Staff users are *not* exempted from library content pathways. """ - VERSION = 1 + WRITE_VERSION = 1 + READ_VERSION = 1 @classmethod def name(cls): diff --git a/lms/djangoapps/course_blocks/transformers/split_test.py b/lms/djangoapps/course_blocks/transformers/split_test.py index d11b8a2dec..9a3dd1231c 100644 --- a/lms/djangoapps/course_blocks/transformers/split_test.py +++ b/lms/djangoapps/course_blocks/transformers/split_test.py @@ -19,7 +19,8 @@ class SplitTestTransformer(FilteringTransformerMixin, BlockStructureTransformer) to actually enforce the access using the 'user_partitions' and 'group_access' fields. """ - VERSION = 1 + WRITE_VERSION = 1 + READ_VERSION = 1 @classmethod def name(cls): diff --git a/lms/djangoapps/course_blocks/transformers/start_date.py b/lms/djangoapps/course_blocks/transformers/start_date.py index 5a8ccb09b1..90822e72ba 100644 --- a/lms/djangoapps/course_blocks/transformers/start_date.py +++ b/lms/djangoapps/course_blocks/transformers/start_date.py @@ -24,7 +24,8 @@ class StartDateTransformer(FilteringTransformerMixin, BlockStructureTransformer) Staff users are exempted from visibility rules. """ - VERSION = 1 + WRITE_VERSION = 1 + READ_VERSION = 1 MERGED_START_DATE = 'merged_start_date' @classmethod diff --git a/lms/djangoapps/course_blocks/transformers/user_partitions.py b/lms/djangoapps/course_blocks/transformers/user_partitions.py index 964a6bedc3..b3635c43f1 100644 --- a/lms/djangoapps/course_blocks/transformers/user_partitions.py +++ b/lms/djangoapps/course_blocks/transformers/user_partitions.py @@ -16,7 +16,8 @@ class UserPartitionTransformer(FilteringTransformerMixin, BlockStructureTransfor Staff users are *not* exempted from user partition pathways. """ - VERSION = 1 + WRITE_VERSION = 1 + READ_VERSION = 1 @classmethod def name(cls): diff --git a/lms/djangoapps/course_blocks/transformers/visibility.py b/lms/djangoapps/course_blocks/transformers/visibility.py index 1fad7ff57e..162541cdaa 100644 --- a/lms/djangoapps/course_blocks/transformers/visibility.py +++ b/lms/djangoapps/course_blocks/transformers/visibility.py @@ -18,7 +18,8 @@ class VisibilityTransformer(FilteringTransformerMixin, BlockStructureTransformer Staff users are exempted from visibility rules. """ - VERSION = 1 + WRITE_VERSION = 1 + READ_VERSION = 1 MERGED_VISIBLE_TO_STAFF_ONLY = 'merged_visible_to_staff_only' diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index bc669d4b45..a2330ff2e7 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -13,6 +13,7 @@ from mock import patch, MagicMock import pytz from util.date_utils import to_timestamp +from openedx.core.lib.block_structure.exceptions import BlockStructureNotFound from student.models import anonymous_id_for_user from student.tests.factories import UserFactory from track.event_transaction_utils import ( @@ -130,7 +131,7 @@ class RecalculateSubsectionGradeTest(ModuleStoreTestCase): self.assertTrue(PersistentGradesEnabledFlag.feature_enabled(self.course.id)) with patch( 'openedx.core.lib.block_structure.factory.BlockStructureFactory.create_from_cache', - return_value=None, + side_effect=BlockStructureNotFound, ) as mock_block_structure_create: self._apply_recalculate_subsection_grade() self.assertEquals(mock_block_structure_create.call_count, 1) diff --git a/lms/djangoapps/grades/transformer.py b/lms/djangoapps/grades/transformer.py index b812c617ed..f481bd2665 100644 --- a/lms/djangoapps/grades/transformer.py +++ b/lms/djangoapps/grades/transformer.py @@ -2,17 +2,13 @@ Grades Transformer """ from base64 import b64encode -from django.test.client import RequestFactory from functools import reduce as functools_reduce from hashlib import sha1 from logging import getLogger import json -from courseware.model_data import FieldDataCache -import courseware.module_render from lms.djangoapps.course_blocks.transformers.utils import collect_unioned_set_field, get_field_on_block from openedx.core.lib.block_structure.transformer import BlockStructureTransformer -from openedx.core.djangoapps.util.user_utils import SystemUser log = getLogger(__name__) @@ -39,7 +35,8 @@ class GradesTransformer(BlockStructureTransformer): max_score: (numeric) """ - VERSION = 4 + WRITE_VERSION = 4 + READ_VERSION = 4 FIELDS_TO_COLLECT = [u'due', u'format', u'graded', u'has_score', u'weight', u'course_version', u'subtree_edited_on'] EXPLICIT_GRADED_FIELD_NAME = 'explicit_graded' diff --git a/openedx/core/lib/block_structure/block_structure.py b/openedx/core/lib/block_structure/block_structure.py index af2b5de0e7..80f32bdc52 100644 --- a/openedx/core/lib/block_structure/block_structure.py +++ b/openedx/core/lib/block_structure/block_structure.py @@ -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): """ diff --git a/openedx/core/lib/block_structure/exceptions.py b/openedx/core/lib/block_structure/exceptions.py index 8d17282464..4debcd0670 100644 --- a/openedx/core/lib/block_structure/exceptions.py +++ b/openedx/core/lib/block_structure/exceptions.py @@ -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 diff --git a/openedx/core/lib/block_structure/factory.py b/openedx/core/lib/block_structure/factory.py index c180b91dd7..0c6c0d187a 100644 --- a/openedx/core/lib/block_structure/factory.py +++ b/openedx/core/lib/block_structure/factory.py @@ -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): diff --git a/openedx/core/lib/block_structure/manager.py b/openedx/core/lib/block_structure/manager.py index 7cada6ac9f..05555bc0ff 100644 --- a/openedx/core/lib/block_structure/manager.py +++ b/openedx/core/lib/block_structure/manager.py @@ -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): diff --git a/openedx/core/lib/block_structure/tests/helpers.py b/openedx/core/lib/block_structure/tests/helpers.py index 4ff4c13dc7..ff9c3b39ed 100644 --- a/openedx/core/lib/block_structure/tests/helpers.py +++ b/openedx/core/lib/block_structure/tests/helpers.py @@ -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): diff --git a/openedx/core/lib/block_structure/tests/test_block_structure.py b/openedx/core/lib/block_structure/tests/test_block_structure.py index b7fb807d7d..e9f27e3b3c 100644 --- a/openedx/core/lib/block_structure/tests/test_block_structure.py +++ b/openedx/core/lib/block_structure/tests/test_block_structure.py @@ -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( diff --git a/openedx/core/lib/block_structure/tests/test_factory.py b/openedx/core/lib/block_structure/tests/test_factory.py index 8ccb5a6207..e798c6f253 100644 --- a/openedx/core/lib/block_structure/tests/test_factory.py +++ b/openedx/core/lib/block_structure/tests/test_factory.py @@ -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( diff --git a/openedx/core/lib/block_structure/tests/test_manager.py b/openedx/core/lib/block_structure/tests/test_manager.py index 6dd4f0f927..bac02cabfc 100644 --- a/openedx/core/lib/block_structure/tests/test_manager.py +++ b/openedx/core/lib/block_structure/tests/test_manager.py @@ -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) diff --git a/openedx/core/lib/block_structure/tests/test_transformers.py b/openedx/core/lib/block_structure/tests/test_transformers.py index 5f712ee343..af68c00072 100644 --- a/openedx/core/lib/block_structure/tests/test_transformers.py +++ b/openedx/core/lib/block_structure/tests/test_transformers.py @@ -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)) diff --git a/openedx/core/lib/block_structure/transformer.py b/openedx/core/lib/block_structure/transformer.py index a16731d981..d51ca46a1b 100644 --- a/openedx/core/lib/block_structure/transformer.py +++ b/openedx/core/lib/block_structure/transformer.py @@ -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): diff --git a/openedx/core/lib/block_structure/transformers.py b/openedx/core/lib/block_structure/transformers.py index 04deaef374..7c5347c02e 100644 --- a/openedx/core/lib/block_structure/transformers.py +++ b/openedx/core/lib/block_structure/transformers.py @@ -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): """