Refactor to authoring package

[ormsbee]

All the Django apps that were in openedx_learning.core were specific to authoring library and course content: publishing, contents, components. As time goes on, we will likely add other groupings of apps, e.g. "learner", "personalization", "grading", etc.

Important compatibility notes:

1. This is a breaking change, and edx-platform will need to be updated to point to the new locations for apps.
2. This should not affect the table names or migrations, since all app names remain the same, and we use the explicit repo-level "oel_" prefix anyway.

[kdmccormick]

@kdmccormick : are components really an "authoring" thing? Isn't LMS going to heavily reference components and contents, in the context of rendering them for learners?

@ormsbee : Yeah, I was thinking about that, and I managed to convince myself that (a) it's okay if the LMS wants to query "the authored content" , but (b) that most LMS access on behalf of a user will probably come through another package like "personalization" which will pull authored content (+ mix it with student data). I think we've talked about "authoring" strictly in the sense of "this is Studio", but I think it should mean, "read/write the content as authored by the course team"

Okay, that makes sense.

I'm always a little hesitant to add more nesting; can you say more about the rationale for having apps/authoring/ instead of apps/components, apps/content/, and apps/publishing/ at the same level as future packages like apps/grading/?

Assuming that we are going forward with apps/authoring/, though, I like that you're consolidating the Python APIs so that most consumers won't need to think about the layers:

As I'm looking in the various places to update this in edx-platform, I had the thought that we could consolidate the Python APIs at the package level. So for instance, if you were importing from edx-platform, you would do:

from openedx_learning.apps.authoring import create_learning_package, create_component

Apps internal to the package would still import from each other so we can still use the import linter to keep our dependencies clear. It makes a little more work for making learning core apps, but I think it will be a better experience for consumers.

Will we remove or rename the underlying ./apps/authoring/SUBPACKAGE/api.py modules, so that we don't have consumers importing from a mix of apps/authoring/api.py and the subpackage api.pys?

[ormsbee]

I'm always a little hesitant to add more nesting; can you say more about the rationale for having apps/authoring/ instead of apps/components, apps/content/, and apps/publishing/ at the same level as future packages like apps/grading/?

Sure. I figure we're going to add at least a few more apps under the authoring umbrella, like units, sequences, assets, export, etc. But then the next group of things we'll want to add are things that relate to a particular learner, and the personalization of content to that learner–things like user partitions, scheduling, etc. Another longer term goal would be grading, around which there are a number of moving pieces that should be modeled separately–e.g. the raw scores, the grading policies, overrides, manual grading, team grades, etc. Or possibly those get folded up into an umbrella of "user activity" to include things like completion.

In any case, I think that at that point, we have at least a few broad packages:

• authoring
• learner
• personalization
• activity (or grading)

I think that two layers of hierarchy gives us a decent tradeoff in terms of making apps that are small enough to work on and understand, while still being able to present clients with a small number of API modules based on logical groupings of functionality.

Will we remove or rename the underlying ./apps/authoring/SUBPACKAGE/api.py modules, so that we don't have consumers importing from a mix of apps/authoring/api.py and the subpackage api.pys?

I think I'd like to use both, but for slightly different things. I'm honestly not sure how this will really pan out, but I thought we might have app api.py guidelines like the following:

1. Specify an __all__ entry to indicate which functions should be publicly re-exported through the top level authoring/api.py module.
2. Functions that are meant to be purely internal to your app's api.py should be prefaced with an underscore (_).
3. Functions that are not in __all__ and are not prefaced with an underscore are meant to be usable by other apps in the authoring package, but are not a part of the promised external API.
4. App api.py files should import from each other, but they should not try to import from the top level api.py. So ./components/api.py can import from ./publishing/api.py, but it should never import from authoring/api.py. These imports should always be explicit and not use wildcard imports.
5. Import dependencies should be explicitly tracked in our top level .importlinter file.

So the authoring/api.py file would blanket re-export the things individual apps declare as their public API, and probably implement some convenience functions that call to multiple app api.py files. But code that's outside of the authoring package only ever imports authoring/api.py, and not any of the sub-package api.py files. I think that gives us more flexibility to refactor later without breaking any contracts, and a simpler story for clients so that they're not in a sea of dozens of api.py files with their various dependencies.

[ormsbee]

@kdmccormick: In Slack, you mentioned:

I guess I always viewed the .api decision as a thing we do for edx-platform & microservices, not for published packages. It strikes me as super unidomatic to have every pacakge import path end in .api . I can't think of a single other Python package I've used that does thing that way.
(...)
I'm regretful that we use .api instead of _ , but it's been OEP'd so it is what it is

That got me thinking. In this PR, I'm currently aggregating the various API modules into openedx_learning.apps.authoring.api, but that doesn't have to be the case. I could just as easily aggregate them in openedx_learning.api.authoring or openedx_learning_api.authoring. OEP-49 lays out conventions for Django apps, but this sort of aggregation layer is a new thing, and I think it's okay to try a new approach and ADR it.

The more I think about it, the more I like this sort of top-level separation of the public part of the API that openedx_learning exposes, instead of mixing them into the hierarchies with apps.

[bradenmacdonald]

My only question so far is pretty minor: Is the apps. namespace adding much value here?

Your proposal:

openedx_learning
openedx_learning.api.authoring
openedx_learning.apps.authoring.(various apps)
openedx_learning.contrib.media_server
openedx_learning.lib.(various)

vs

openedx_learning
openedx_learning.api.authoring
openedx_learning.authoring.(various apps)
openedx_learning.contrib.media_server
openedx_learning.lib.(various)

[ormsbee]

@bradenmacdonald: Fair point, esp. since I want to also kill the contrib package when we do the assets support for real (and at any rate, it would probably go under apps in this arrangement).

Still, even if there are only a few things in it, I like having that layer of apps. It hopefully makes it obvious that all Django apps go in there, and that we should not have them in openedx_learning.lib or openedx_learning.api. It makes it easier to make blanket statements like, "things in edx-platform can import from api and lib, but cannot grab from apps".

But at the end of the day, I think I just like the uniformity where api and lib are top level peers of apps, and not not mixed in as peers of different groups of apps like authoring.

[ormsbee]

I disabled auto-generation of all API docs here. There's only a manual generation of docs for public APIs and models, via the /docs/public_apis dir. The generated docs are usable but not pretty (a lot of formatting issues). But I figure those could be for later.

[ormsbee]

@kdmccormick, @bradenmacdonald: This PR should be in a reviewable state.

[kdmccormick]

Looks great! 🚀

[kdmccormick]

accidental?

Suggested change

	# doc8 --ignore-path docs/_build README.rst docs
	doc8 --ignore-path docs/_build README.rst docs

[ormsbee]

Oh, right. I couldn't build the docs with that in there because we aren't building docs properly in CI and there were syntax linting issues in some of the ADRs. I meant to fix that in a separate commit after everything else was passing. I'll do that now.

[ormsbee]

Okay, this is fixed up. There's definitely more docs cleanup that needs to be done later as a followup.

[ormsbee]

I'm holding off of merging this until openedx/edx-platform#34796 lands, since that is intended for backport into Redwood, and these changes are incompatible with the edx-platform code cut in Redwood.