Upgrading Tutor for Koa

As part of the Koa release schedule, I’m working on getting Tutor to work with the latest master branches of Open edX. Similarly to this other topic I’m putting my notes here in case other people face the same issues. This is mostly a brain dump, where I’m thinking out loud.

When collecting static assets I face the following error coming from the edx-ora2 xblock:

# openedx-assets collect --settings=tutor.assets
Unable to load XBlock 'openassessment'
Traceback (most recent call last):
  File "/openedx/venv/lib/python3.8/site-packages/xblock/plugin.py", line 141, in load_classes
    yield (class_.name, cls._load_class_entry_point(class_))
  File "/openedx/venv/lib/python3.8/site-packages/xblock/plugin.py", line 70, in _load_class_entry_point
    class_ = entry_point.load()
  File "/openedx/venv/lib/python3.8/site-packages/pkg_resources/__init__.py", line 2443, in load
    return self.resolve()
  File "/openedx/venv/lib/python3.8/site-packages/pkg_resources/__init__.py", line 2449, in resolve
    module = __import__(self.module_name, fromlist=['__name__'], level=0)
  File "/openedx/venv/lib/python3.8/site-packages/openassessment/xblock/openassessmentblock.py", line 101, in <module>
    class OpenAssessmentBlock(MessageMixin,
  File "/openedx/venv/lib/python3.8/site-packages/openassessment/xblock/openassessmentblock.py", line 494, in OpenAssessmentBlock
    def student_view(self, context=None):  # pylint: disable=unused-argument
  File "/openedx/venv/lib/python3.8/site-packages/openassessment/xblock/mobile.py", line 19, in __call__
    if self.is_mobile_support_waffle_enabled:
  File "/openedx/venv/lib/python3.8/site-packages/django/utils/functional.py", line 80, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
  File "/openedx/venv/lib/python3.8/site-packages/openassessment/xblock/config_mixin.py", line 121, in is_mobile_support_waffle_enabled
    return self.is_feature_enabled(MOBILE_SUPPORT)
  File "/openedx/venv/lib/python3.8/site-packages/openassessment/xblock/config_mixin.py", line 87, in is_feature_enabled
    if self._waffle_switch(flag).is_enabled():
  File "/openedx/edx-platform/openedx/core/djangoapps/waffle_utils/__init__.py", line 231, in is_enabled
    return self.waffle_namespace.is_enabled(self.switch_name)
  File "/openedx/edx-platform/openedx/core/djangoapps/waffle_utils/__init__.py", line 133, in is_enabled
    value = switch_is_active(namespaced_switch_name)
  File "/openedx/venv/lib/python3.8/site-packages/waffle/__init__.py", line 21, in switch_is_active
    switch = Switch.get(switch_name)
  File "/openedx/venv/lib/python3.8/site-packages/waffle/models.py", line 46, in get
    obj = cls.get_from_db(name)
  File "/openedx/venv/lib/python3.8/site-packages/waffle/models.py", line 59, in get_from_db
    return objects.get(name=name)
  File "/openedx/venv/lib/python3.8/site-packages/django/db/models/manager.py", line 82, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/openedx/venv/lib/python3.8/site-packages/django/db/models/query.py", line 402, in get
    num = len(clone)
  File "/openedx/venv/lib/python3.8/site-packages/django/db/models/query.py", line 256, in __len__
    self._fetch_all()
  File "/openedx/venv/lib/python3.8/site-packages/django/db/models/query.py", line 1242, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/openedx/venv/lib/python3.8/site-packages/django/db/models/query.py", line 55, in __iter__
    results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
  File "/openedx/venv/lib/python3.8/site-packages/django/db/models/sql/compiler.py", line 1129, in execute_sql
    sql, params = self.as_sql()
  File "/openedx/venv/lib/python3.8/site-packages/django/db/models/sql/compiler.py", line 474, in as_sql
    extra_select, order_by, group_by = self.pre_sql_setup()
  File "/openedx/venv/lib/python3.8/site-packages/django/db/models/sql/compiler.py", line 54, in pre_sql_setup
    self.setup_query()
  File "/openedx/venv/lib/python3.8/site-packages/django/db/models/sql/compiler.py", line 45, in setup_query
    self.select, self.klass_info, self.annotation_col_map = self.get_select()
  File "/openedx/venv/lib/python3.8/site-packages/django/db/models/sql/compiler.py", line 254, in get_select
    sql, params = self.compile(col, select_format=True)
  File "/openedx/venv/lib/python3.8/site-packages/django/db/models/sql/compiler.py", line 405, in compile
    sql, params = node.as_sql(self, self.connection)
  File "/openedx/venv/lib/python3.8/site-packages/django/db/models/expressions.py", line 737, in as_sql
    return "%s.%s" % (qn(self.alias), qn(self.target.column)), []
  File "/openedx/venv/lib/python3.8/site-packages/django/db/models/sql/compiler.py", line 396, in quote_name_unless_alias
    r = self.connection.ops.quote_name(name)
  File "/openedx/venv/lib/python3.8/site-packages/django/db/backends/dummy/base.py", line 20, in complain
    raise ImproperlyConfigured("settings.DATABASES is improperly configured. "
django.core.exceptions.ImproperlyConfigured: settings.DATABASES is improperly configured. Please supply the ENGINE value. Check settings documentation for more details.
                Finished collecting cms assets.

It appears that the openassessment xblock cannot collect static assets without a MySQL database connection. This is unfortunate, as it prevents static asset collection at build time.

The error stems from the @togglable_mobile_support decorator which is evaluated at import time. This decorator calls the is_mobile_support_waffle_enabled method, which itself calls the WaffleSwitch(...).is_enabled() method. Sorry to be pointing fingers, but this was introduced by @morenol here: https://github.com/edx/edx-ora2/pull/1445

When I add return True at the top of the TogglableMobileSupport.__call__ method the error disappears.

@morenol what are your thoughts on this? Do we want to get rid of that waffle flag? After all, we can assume that everyone wants to display openassessment units in their mobile apps, right? Defining the ENABLE_ORA_MOBILE_SUPPORT feature setting would not resolve anything, because the setting check is performed after the waffle switch check.

Hi @regis,

I was not aware that this could affect this :disappointed: I think that in the long term, we should remove that feature flag. In the short term, I think that we can solve this by removing the usage of waffle and changing this to only use the _settings_toggle_enabled method:

Unfortunately as far as I have seen we also have call the XBlock.supports(‘multi_device’) at import time, for that reason I had to add the calls to waffle also at import, but maybe we can remove those calls and depend for now only in the FEATURES setting until we decide to enable that for everyone.

What do you think?

Thanks for your answer @morenol!

I think that we should get rid of the waffle flag. Waffle flags are supposed to have a target removal date, as documented here: https://edx-toggles.readthedocs.io/en/latest/how_to/documenting_new_feature_toggles.html (disclaimer: I wrote this how to)
In this particular case, my understanding is that the point of this waffle flag was only to check whether the feature was working as expected. It’s been in production for a few months now, so I expect that the feature works. Thus, we should get rid of the waffle flag. If for some weird reason some platforms do not want to display open assessment xblocks in the mobile app, they can always decide to disable the feature via the setting. (By the way, I also think that the default value for the feature setting should be to enable the feature.)

@morenol Do you have time and are you willing to take this issue or should I assign this to myself? https://openedx.atlassian.net/jira/software/projects/BTR/boards/641?selectedIssue=BTR-26

Assign that to me, I can create a PR today. I will remove the waffle flag but I will keep the default still disabled, I think that edX does not have that enabled. In particular this is required for the usage of ORA in iOS and have not been merged yet.

1 Like

As anyone managed to get Notes to work with open-release/koa.test01? I’m getting the following stacktrace when attempting to save a new note:

notes_1          | POST http://elasticsearch:9200/_bulk?refresh=true [status:400 request:0.002s]                                                     [32/1413]
notes_1          | Internal Server Error: /api/v1/annotations/1/                                                                                              
notes_1          | Traceback (most recent call last):                                                                                                         
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django/core/handlers/exception.py", line 34, in inner                                       
notes_1          |     response = get_response(request)                                                                                                       
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django/core/handlers/base.py", line 115, in _get_response                                   
notes_1          |     response = self.process_exception_by_middleware(e, request)                                                                            
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django/core/handlers/base.py", line 113, in _get_response                                   
notes_1          |     response = wrapped_callback(request, *callback_args, **callback_kwargs)                                                                
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django/views/decorators/csrf.py", line 54, in wrapped_view                                  
notes_1          |     return view_func(*args, **kwargs)                                                                                                      
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django/views/generic/base.py", line 71, in view                                             
notes_1          |     return self.dispatch(request, *args, **kwargs)                                                                                         
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/rest_framework/views.py", line 505, in dispatch                                             
notes_1          |     response = self.handle_exception(exc)                                                                                                  
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/rest_framework/views.py", line 465, in handle_exception                                     
notes_1          |     self.raise_uncaught_exception(exc)                                                                                                     
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/rest_framework/views.py", line 476, in raise_uncaught_exception                             
notes_1          |     raise exc                                                                                                                              
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/rest_framework/views.py", line 502, in dispatch                                             
notes_1          |     response = handler(request, *args, **kwargs)                                                                                           
notes_1          |   File "/openedx/edx-notes-api/notesapi/v1/views.py", line 559, in delete                                                                  
notes_1          |     note.delete()                                                                                                                          
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django/db/models/base.py", line 919, in delete                                              
notes_1          |     return collector.delete()                                                                                                              
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django/db/models/deletion.py", line 317, in delete                                          
notes_1          |     signals.post_delete.send(                                                                                                              
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django/dispatch/dispatcher.py", line 173, in send                                           
notes_1          |     return [                                                                                                                               
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django/dispatch/dispatcher.py", line 174, in <listcomp>                                     
notes_1          |     (receiver, receiver(signal=self, sender=sender, **named))                                                                              
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django_elasticsearch_dsl/signals.py", line 72, in handle_delete                        
notes_1          |     registry.delete(instance, raise_on_error=False)                                                                                [0/1413]
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django_elasticsearch_dsl/registries.py", line 148, in delete
notes_1          |     self.update(instance, action="delete", **kwargs)
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django_elasticsearch_dsl/registries.py", line 141, in update
notes_1          |     doc().update(instance, **kwargs)
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django_elasticsearch_dsl/documents.py", line 195, in update
notes_1          |     return self._bulk(
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django_elasticsearch_dsl/documents.py", line 179, in _bulk
notes_1          |     return self.bulk(*args, **kwargs)
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/django_elasticsearch_dsl/documents.py", line 146, in bulk
notes_1          |     return bulk(client=self._get_connection(), actions=actions, **kwargs)
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/elasticsearch/helpers/actions.py", line 360, in bulk
notes_1          |     for ok, item in streaming_bulk(client, actions, *args, **kwargs):
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/elasticsearch/helpers/actions.py", line 281, in streaming_bulk
notes_1          |     for data, (ok, info) in zip(
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/elasticsearch/helpers/actions.py", line 217, in _process_bulk_chunk
notes_1          |     for item in gen:
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/elasticsearch/helpers/actions.py", line 166, in _process_bulk_chunk_error
notes_1          |     raise error
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/elasticsearch/helpers/actions.py", line 205, in _process_bulk_chunk
notes_1          |     resp = client.bulk("\n".join(bulk_actions) + "\n", *args, **kwargs)
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/elasticsearch/client/utils.py", line 139, in _wrapped
notes_1          |     return func(*args, params=params, headers=headers, **kwargs)
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/elasticsearch/client/__init__.py", line 428, in bulk
notes_1          |     return self.transport.perform_request(
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/elasticsearch/transport.py", line 345, in perform_request
notes_1          |     status, headers_response, data = connection.perform_request(
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/elasticsearch/connection/http_urllib3.py", line 256, in perform_request
notes_1          |     self._raise_error(response.status, raw_data)
notes_1          |   File "/usr/local/lib/python3.8/dist-packages/elasticsearch/connection/base.py", line 287, in _raise_error
notes_1          |     raise HTTP_EXCEPTIONS.get(status_code, TransportError)(
notes_1          | elasticsearch.exceptions.RequestError: RequestError(400, 'ActionRequestValidationException[Validation Failed: 1: type is missing;]', 'ActionRequestValidationException[Validation Failed: 1: type is missing;]')

EDIT: this PR leads me to believe that notes was upgraded to use Elasticsearch 7. If this is true we will either have to make notes work with ES 1.5.2 or flag notes as incompatible with Koa :-/

EDIT 2: a third option would be to tag an earlier version of edx-notes-api for release with Koa. According to my experiments commit cc29e31 is compatible with the open-release/koa.test01 version of edx-platform.

We had tried to use an ES1.5-compatible commit in edx-notes-api, but maybe we got the wrong one? You should have commit 03cdb12 for edx-notes-api.

Though now that I look at it more closely, 03cdb12 has ES7 upgrade work in its ancestry.

@Diana_Huang Can you help us untangle this?

Yeah, I probably used the wrong base commit to cherry-pick others onto. I’ll try to find some time today to go back in and pull the correct commits out.

1 Like

I updated the cherry-picked Koa branch, which should hopefully work now. :crossed_fingers: https://github.com/edx/edx-notes-api/pull/220

Thanks! I’ve changed koa.master and koa.test01 in edx-notes-api to point to this new commit: 1ab55c4

Thanks a bunch for your reactivity @Diana_Huang and @nedbat, I appreciate it :slight_smile: I can confirm that the new koa.test01 branch works. for edx-notes-api.

3 Likes