Skip to content

Commit

Permalink
fixup!: get rid of some confusing experimentation cruft
Browse files Browse the repository at this point in the history
  • Loading branch information
ormsbee committed Nov 16, 2023
1 parent 6bdacf9 commit bb50ffb
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 39 deletions.
13 changes: 13 additions & 0 deletions openedx/core/djangoapps/xblock/runtime/blockstore_field_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,19 @@ def _check_field(self, block, name):
# it's mostly relevant as Scope.preferences(UserScope.ONE, BlockScope.TYPE)
# Which would be handled by a user-aware FieldData implementation

def _get_active_block(self, block):
"""
Get the ActiveBlock entry for the specified block, creating it if
necessary.
"""
key = get_weak_key_for_block(block)
if key not in self.active_blocks:
self.active_blocks[key] = ActiveBlock(
olx_hash=get_olx_hash_for_definition_key(block.scope_ids.def_id),
changed_fields={},
)
return self.active_blocks[key]

def get(self, block, name):
"""
Get the given field value from Blockstore
Expand Down
3 changes: 1 addition & 2 deletions openedx/core/djangoapps/xblock/runtime/blockstore_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,13 @@ class BlockstoreXBlockRuntime(XBlockRuntime):
def parse_xml_file(self, fileobj, id_generator=None):
raise NotImplementedError("Use parse_olx_file() instead")

def get_block(self, usage_key, for_parent=None):
def get_block(self, usage_id, for_parent=None):
"""
Create an XBlock instance in this runtime.
Args:
usage_key(OpaqueKey): identifier used to find the XBlock class and data.
"""
usage_id = usage_key
def_id = self.id_reader.get_definition_id(usage_id)
if def_id is None:
raise ValueError(f"Definition not found for usage {usage_id}")
Expand Down
48 changes: 11 additions & 37 deletions openedx/core/djangoapps/xblock/runtime/learning_core_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,35 +122,9 @@ def get(self, block, name):
block.scope_ids.usage_id,
name,
)
# If 'name' is not found, this will raise KeyError, which means to use the default value
raise

# Otherwise, check to see if it
entry = self._get_active_block(block)
if name in entry.changed_fields:
value = entry.changed_fields[name]
if value == DELETED:
raise KeyError # KeyError means use the default value, since this field was deliberately set to default
return value
try:
saved_fields = self.loaded_definitions[entry.olx_hash]
except KeyError:
if name == CHILDREN_INCLUDES:
# Special case: parse_xml calls add_node_as_child which calls 'block.children.append()'
# BEFORE parse_xml is done, and .append() needs to read the value of children. So
return [] # start with an empty list, it will get filled in.
# Otherwise, this is an anomalous get() before the XML was fully loaded:
# This could happen if an XBlock's parse_xml() method tried to read a field before setting it,
# if an XBlock read field data in its constructor (forbidden), or if an XBlock was loaded via
# some means other than runtime.get_block(). One way this can happen is if you log/print an XBlock during
# XML parsing, because ScopedStorageMixin.__repr__ will try to print all field values, and any fields which
# aren't mentioned in the XML (which are left at their default) will be "not loaded yet."
log.exception(
"XBlock %s tried to read from field data (%s) that wasn't loaded from Blockstore!",
block.scope_ids.usage_id, name,
)
raise # Just use the default value for now; any exception raised here is caught anyways
return saved_fields[name]
# If 'name' is not found, this will raise KeyError, which means to use the default value

def has_changes(self, block):
usage_key = block.scope_ids.usage_id
Expand Down Expand Up @@ -197,6 +171,12 @@ def default(self, block, name):


class LearningCoreDefinitionLocator(CheckFieldMixin, DefinitionKey):
"""
Skeleton of a new definition locator, in case we need it.
Hopefully this isn't necessary at all. So far just passing Nones for def
keys and ignoring the def key elsewhere seems to be working out.
"""
CANONICAL_NAMESPACE = 'oel-cv' # openedx-learning component version
KEY_FIELDS = ('uuid',)
#uuid = UUID
Expand All @@ -208,17 +188,11 @@ def __init__(self, uuid: UUID | str):

class LearningCoreOpaqueKeyReader(OpaqueKeyReader):
def get_definition_id(self, usage_id):
"""
This is mostly here to make sure LearningCore-based things *don't* call
it. By making it explode if it's called.
"""
1/0
if not isinstance(usage_id, UsageKeyV2):
raise TypeError(
f"This version of get_definition_id doesn't support the key type for {usage_id}."
)

return get_learning_context_impl(usage_id).definition_for_usage(usage_id)

class NullDefinitionKey(DefinitionKey):
"""This isn't really used."""



class LearningCoreXBlockRuntime(BlockstoreXBlockRuntime):
Expand Down

0 comments on commit bb50ffb

Please sign in to comment.