Plugins and {common,lms}-env-features: shouldn't the patch() invocations be at the end of the FEATURES dictionary?

Tutor Plugins
1

Hi everyone,

currently, Tutor generates the FEATURES option in lms.env.json like this:

  "FEATURES": {
    {{ patch("common-env-features", separator=",\n", suffix=",")|indent(4) }}
    {{ patch("lms-env-features", separator=",\n", suffix=",")|indent(4) }}
    "CERTIFICATES_HTML_VIEW": true,
    "PREVIEW_LMS_BASE": "{{ PREVIEW_LMS_HOST }}",
    "ENABLE_CORS_HEADERS": true,
    "ENABLE_COURSE_DISCOVERY": true,
    "ENABLE_COURSEWARE_SEARCH": true,
    "ENABLE_CSMH_EXTENDED": false,
    "ENABLE_DASHBOARD_SEARCH":  true,
    "ENABLE_COMBINED_LOGIN_REGISTRATION": true,
    "ENABLE_GRADE_DOWNLOADS": true,
    "ENABLE_LEARNER_RECORDS": false,
    "ENABLE_MOBILE_REST_API": true,
    "ENABLE_OAUTH2_PROVIDER": true,
    "ENABLE_THIRD_PARTY_AUTH": true,
    "MILESTONES_APP": true,
    "ENABLE_PREREQUISITE_COURSES": true
  },

So if I’m not mistaken, that means that if a plugin wants to override any of the settings that Tutor hardcodes (say, setting ENABLE_GRADE_DOWNLOADS to false in the lms-env-features patch), it’s the hardcoded definition that comes last and therefore applies.

Maybe I’m missing something, but shouldn’t the patch() invocations move from the top to the bottom of the FEATURES dictionary? Or, else, should there be configuration settings for all these hardcoded values, so that no plugin is needed to set them?

Cheers,
Florian

1 Like
2

Hi @fghaas ,
I believe you can do that with two way approach:
the first one is with a YAML plugin, like this: (nano .local/share/tutor-plugins/just_test.yml)

 ---
 name: just_test
 version: 0.1.0
 patches:
     lms-env-features: |
        "ENABLE_GRADE_DOWNLOADS": false

OR:

from command line:

tutor config save --set ENABLE_GRADE_DOWNLOADS=False (Not sure for this)

Murat

3

Go ahead and try that, and see if it actually disables grade downloads. I think it won’t, because your lms.env.json will now contain

FEATURES: {
  "ENABLE_GRADE_DOWNLOADS": false,
  [...]
  "ENABLE_GRADE_DOWNLOADS": true,
  [...]
}

… and the second setting will count. No?

4

Right! just tested and its duplicated as you mentioned. :+1:

5

Yup, so, original question still stands. :smiley:

6

In general, I think it’s better if the *-env-features patches are not implemented. I have no idea how the Python json loader interprets identical keys with different values. Instead, you should enable the features from the settings patches. Remember that in such cases the patch should be formatted as Python code. For instance:

patches:
     openedx-lms-common-settings: |
       FEATURES["ENABLE_GRADE_DOWNLOADS"] = False
7

OK, that’s a bit of a surprise — the documentation doesn’t really say anything about some patches being preferred over others, so it’s not straightforward for plugin authors to discern which patches they should use and which they had rather not.

So is that a general recommendation? When in doubt, don’t use *-env-features, use *-settings instead?

8

Great question @fghaas

As I am in the process of testing the migration of our Native environment to the Tutor environment, I was wondering about this because we have a lot of FEATURES activated that are not in the basic FEATURES available in the $(tutor config printroot)/config.yml file.

It wasn’t clear to me if I needed to include them in the $(tutor config printroot)/config.yml file or directly in the lms.env.json or cms.env.json files. From what I now understand, the json files would be overwritten the next time I rebuild my environment.

Which brings the question, how do separate the FEATURES for the lms from those for the cms in the config file?

9

I set all the settings parameters in the python settings files and ignore the json config altogether (unless I really have to).

There are patches for different levels (common, cms only, lms only, lms prod only etc…). All you would need is a YAML plugin.

I have an example here with a python plugin.

Edit: Just saw this thread :slight_smile:

2 Likes
10

Yes, you’re right Florian. The documentation does not really state that as a fact because it’s more of a personal opinion of mine. It’s just that I find that is much easier to patch python modules than json (or even yaml) files.
Actually, I am planning on migrating the lms/cms.env.json files to yaml in the Nutmeg release. We should take that opportunity to add a big warning at the top of the file: “Are you quite sure that you want to make changes to this file and not to the Python settings?” and then list all of the reasons why patching these files is not such a great idea.

11

This happens to be one of the rather confusing things that struck me when I first started working with Tutor: is there a particular reason why Tutor has stuck to lms.env.json when Open edX (with edx-configuration, pre-Maple) has been building /edx/etc/lms.yml files for a while now?

12

Mostly plugin backward compatibility. But it’ s high time that we migrate to yaml files, and I’d like to do that in Nutmeg.