The hooks API had several issues which are summarized in this comment:
https://github.com/openedx/wg-developer-experience/issues/125#issuecomment-1313553526
1. "consts" was a bad name
2. "hooks.filters" and "hooks.Filters" could easily be confused
3. docs made it difficult to understand that plugin developers should use the catalog
To address these issues, we:
1. move "consts.py" to "catalog.py"
2. Remove "hooks.actions", "hooks.filters", "hooks.contexts" from the API.
3. re-organize the docs and give better usage examples in the catalog.
This change is a partial fix for https://github.com/openedx/wg-developer-experience/issues/125
Adds `from __future__ import annotations` to the top of every module,
right below the module's docstring. Replaces any usages of t.List,
t.Dict, t.Set, t.Tuple, and t.Type with their built-in equivalents:
list, dict, set, tuple, and type. Ensures that make test still passes
under Python 3.7, 3.8 and 3.9.
When typing `tutor local run --mount /path/to/edx-pl<TAB>`, the mount option
should be auto-completed to the full edx-platform repo path. That is, if shell
completion is enabled:
https://docs.tutor.overhang.io/install.html#shell-autocompletion
Here, we make sure that the implicit form of the `--mount` argument is properly
auto-completed. We are unable to get completion to work in the explicit form,
because args that include colons do not even reach the `shell_completion`
method.
Now that the mypy bugs have been resolved, we are able to define more precisely
and cleanly the types of Actions and Filters.
Moreover, can now strongly type named actions and hooks (in consts.py). With
such a strong typing, we get early alerts of hooks called with incorrect
arguments, which is nothing short of awesome :)
This change breaks the hooks API by removing the `context=...` argument. The
reason for that is that we cannot insert arbitrary arguments between `P.args,
P.kwargs`: https://peps.python.org/pep-0612/#the-components-of-a-paramspec
> A function declared as def inner(a: A, b: B, *args: P.args, **kwargs:
> P.kwargs) -> R has type Callable[Concatenate[A, B, P], R]. Placing
> keyword-only parameters between the *args and **kwargs is forbidden.
Getting the documentation to build in nitpicky mode is quite difficult... We
need to add `nitpick_ignore` to the docs conf.py, otherwise sphinx complains
about many missing class references. This, despite upgrading almost all doc
requirements (except docutils).
We introduce a new filter to implement custom commands in arbitrary containers.
It becomes easy to write convenient ad-hoc commands that users will
then be able to run either on Kubernetes or locally using a documented CLI.
Pluggable jobs are declared as Click commands and are responsible for
parsing their own arguments. See the new CLI_DO_COMMANDS filter.
Close https://github.com/overhangio/2u-tutor-adoption/issues/75
The -m/--mount option makes it possible to bind-mount volumes at runtime. The
volumes are declared in a local/docker-compose.tmp.yml file. The problem with
this approach is when we want to bind-mount a volume to a service which is
specific to the dev context. For instance: the "learning" service when the MFE
plugin is enabled.
In such a case, starting the service triggers a call to `docker-compose stop`
in the local context. This call fails because the "learning" service does not
exist in the local context. Note that this issue only seems to occur with
docker-compose v1.
To resolve this issue, we create two additional filters for
the dev context, which emulate the behaviour of the local context. With this approach, we convert the -m/--mount arguments right after they are parsed. Because they are parsed just once, we can get rid of the de-duplication logic initially introduced with the COMPOSE_CLI_MOUNTS context.
Close #711. Close also https://github.com/overhangio/tutor-mfe/issues/57.
In certain code paths, such as in `tutor local quickstart`,
`process_mount_points` is called more than once in the same process,
causing mounts to be added to `COMPOSE_LOCAL[_JOBS]_TMP` redundantly.
As a result, docker-compose[.jobs].tmp.yml was occasionally being
rendered with duplicate volume specifiers. Some versions of Docker
Compose ignored this; other versions warned or threw an error.
In order to make `process_mount_points` tolerant to being called
multiple times, we wrap its volume-adding callbacks within a new
hooks context. This allows us to clear said hooks context every
time `process_mount_points` is called, essentially making the
function idempotent.
Co-authored-by: Régis Behmo <regis@behmo.com>
Commit 514e3fce22 made it so
that dev containers were to load mounts from
env/dev/docker-compose.tmp.yml. However, it did not update
the code to generates the docker-compose.tmp.yml files.
This manifested as mounts simply not working in dev mode.
Additionally, we make the docker-compose.jobs.tmp.yml files
follow the same local vs dev differentiation that
was introduced in 514e3fce22.
When mounting a directory in a dev-only container, such as the
"learning" mfe, docker-compose is failing because it is attempting to
run "docker-compose stop" in the local context -- which knows nothing
about the learning container.
To resolve this, we store tmp volumes either in the local or dev
docker-compose.yml, and load either one depending on the context.
Running `local start` while a dev platform is still running is a common sourse
of mistakes. Here we introduce a new action to automatically stop local and dev
projects whenever a project with a different name is started.
`copyfrom` copies data from a container to the local filesystem. It's similar
to bindmount, but less clunky, and more intuitive. Also, it plays along great
with `--mount`. Eventually we'll just get rid of the `bindmount` command and
the `--volume` option.
The `--mount` option is available both with `tutor local`
and `tutor dev` commands. It allows users to easily bind-mount containers from
the host to containers. Yes, I know, we already provide that possibility with
the `bindmount` command and the `--volume=/path/` option. But these suffer from
the following drawbacks:
- They are difficult to understand.
- The "bindmount" command name does not make much sense.
- It's not convenient to mount an arbitrary folder from the host to multiple
containers, such as the many lms/cms containers (web apps, celery workers and
job runners).
To address this situation, we now recommend to make use of --mount:
1. `--mount=service1[,service2,...]:/host/path:/container/path`: manually mount
`/host/path` to `/container/path` in container "service1" (and "service2").
2. `--mount=/host/path`: use the new v1 plugin API to discover plugins that
will detect this option and select the right containers in which to bind-mount
volumes. This is really nifty...
Close https://github.com/overhangio/2u-tutor-adoption/issues/43
This is a very large refactoring which aims at making Tutor both more
extendable and more generic. Historically, the Tutor plugin system was
designed as an ad-hoc solution to allow developers to modify their own
Open edX platforms without having to fork Tutor. The plugin API was
simple, but limited, because of its ad-hoc nature. As a consequence,
there were many things that plugin developers could not do, such as
extending different parts of the CLI or adding custom template filters.
Here, we refactor the whole codebase to make use of a generic plugin
system. This system was inspired by the Wordpress plugin API and the
Open edX "hooks and filters" API. The various components are added to a
small core thanks to a set of actions and filters. Actions are callback
functions that can be triggered at different points of the application
lifecycle. Filters are functions that modify some data. Both actions and
filters are collectively named as "hooks". Hooks can optionally be
created within a certain context, which makes it easier to keep track of
which application created which callback.
This new hooks system allows us to provide a Python API that developers
can use to extend their applications. The API reference is added to the
documentation, along with a new plugin development tutorial.
The plugin v0 API remains supported for backward compatibility of
existing plugins.
Done:
- Do not load commands from plugins which are not enabled.
- Load enabled plugins once on start.
- Implement contexts for actions and filters, which allow us to keep track of
the source of every hook.
- Migrate patches
- Migrate commands
- Migrate plugin detection
- Migrate templates_root
- Migrate config
- Migrate template environment globals and filters
- Migrate hooks to tasks
- Generate hook documentation
- Generate patch reference documentation
- Add the concept of action priority
Close #499.
- A shared cookie domain between lms and cms is no longer recommended:
https://github.com/edx/edx-platform/blob/master/docs/guides/studio_oauth.rst
- refactor: clean mounted data folder in lms/cms. In Lilac, the
bind-mounted lms/data and cms/data folders are a mess because new
folders are created there for every new course organisation. These
folders are empty. As far as we know they are useless... With this
change we move these folders to a dedicated "modulestore" subdirectory;
which corresponds better to the initial intent of the fs_root setting.
- fix: frontend failure during login to the lms. See:
https://github.com/openedx/build-test-release-wg/issues/104
- feat: move all forum-related code to a dedicated plugin. Forum is an
optional feature, and as such it deserves its own plugin. Starting from
Maple, users will be able to install the forum from
https://github.com/overhangio/tutor-forum/
- migrate from DCS_* session cookie settings to SESSION_*. That's
because edx-platform no longer depends on django-cookies-samesite. Close
https://github.com/openedx/build-test-release-wg/issues/110
- get rid of tons of deprecation warnings in the lms/cms
- feat: make it possible to point to themed assets. Cherry-picking this
change makes it possible to point to themed assets with a theme-agnostic
url, notably from MFEs.
- Install all official plugins as part of the `tutor[full]` package.
- Don't print error messages about loading plugins during autocompletion.
- Prompt for image building when upgrading from one release to the next.
- Add `tutor local start --skip-build` option to skip building Docker images.
Close #450.
Close #545.
Previously, job declarations were always loaded from local/docker-compose.yml
and local/docker-compose.jobs.yml. This meant that it was not possible to
override job declarations in dev mode. It is now the case, with
dev/docker-compose.jobs.yml and dev/docker-compose.jobs.override.yml. Neither
of these files exist yet... But who knows? we might need this feature one day.
In any case the code is much cleaner now.
Before, custom `docker_compose_func` arguments had to be passed to job runners.
This was not very elegant. Also, it prevented us from loading custom job files
in development.
Here, we adopt a better object-oriented approach, where context classes are
ordered hierarchically.
This paves the way for loading `dev/docker-compose.jobs.yml` files in `tutor
dev init` commands -- which will be necessary to fix permissions in dev/local
mode.
Previously, we were building all images every time we ran a "local start"
command. This was causing unnecessary rebuild. Here, instead, we make use of
the `docker-compose up --build`. This means that only the required images will
be rebuilt.
Previously, the list of domain names to which a theme was assigned had to be
specified manually. Now, the themes are automatically assigned to the LMS and
the CMS, both in development and production modes.
When running `tutor local quickstart -p` we were getting the following error:
Usage: custom [OPTIONS] ARGS...
Try 'custom --help' for help.
Error: Missing argument 'ARGS...'.
The docker-compose command sometimes accept a single command ("pull") with zero
argument.
See: https://discuss.overhang.io/t/local-quickstart-not-working-when-pullimages-enabled/1526
I stumbled upon a bug that should have been detected by the type
checking. Turns out, considering that config is of type Dict[str, Any]
means that we can use just any method on all config values -- which is
terrible. I discovered this after I set `config["PLUGINS"] = None`:
this triggered a crash when I enabled a plugin.
We resolve this by making the Config type more explicit. We also take
the opportunity to remove a few cast statements.
Annotations were generated with pyannotate:
https://github.com/dropbox/pyannotate
We are running in strict mode, which is awesome!
This affects a large part of the code base, which might be an issue for
people running a fork of Tutor. Nonetheless, the behavior should not be
affected. If anything, this process has helped find and resolve a few
type-related bugs. Thus, this is not considered as a breaking change.
It made little sense to create dedicated context classes for local/dev.
Instead, we create local/dev compose methods which can be easily reused.
Also, we renamed the "scripts" module to better reflect its function.
This introduces a new dev/local command:
tutor dev bindmount CONTAINER PATH
And a new volume syntax:
tutor dev run --volume=PATH CONTAINER
This syntax automatically bind-mounts folders from the tutorroot/volumes
directory, which is pretty nifty.
- 💥[Improvement] Upgrade Open edX to Koa
- 💥 Setting changes:
- The ``ACTIVATE_HTTPS`` setting was renamed to ``ENABLE_HTTPS``.
- Other ``ACTIVATE_*`` variables were all renamed to ``RUN_*``.
- The ``WEB_PROXY`` setting was removed and ``RUN_CADDY`` was added.
- The ``NGINX_HTTPS_PORT`` setting is deprecated.
- Architectural changes:
- Use Caddy as a web proxy for automated SSL/TLS certificate generation:
- Nginx no longer listens to port 443 for https traffic
- The Caddy configuration file comes with a new ``caddyfile`` patch for much simpler SSL/TLS management.
- Configuration files for web proxies are no longer provided.
- Kubernetes deployment no longer requires setting up a custom Ingress resource or custom manager.
- Gunicorn and Whitenoise are replaced by uwsgi: this increases boostrap performance and makes it no longer necessary to mount media folders in the Nginx container.
- Replace memcached and rabbitmq by redis.
- Additional features:
- Make it possible to disable all plugins at once with ``plugins disable all``.
- Add ``tutor k8s wait`` command to wait for a pod to become ready
- Faster, more reliable static assets with local memory caching
- Deprecation: proxy files for Apache and Nginx are no longer provided out of the box.
- Removed plugin `{{ patch (...) }}` statements:
- "https-create", "k8s-ingress-rules", "k8s-ingress-tls-hosts": these are no longer necessary. Instead, declare your app in the "caddyfile" patch.
- "local-docker-compose-nginx-volumes": this patch was primarily used to serve media assets. The recommended is now to serve assets with uwsgi.
Running jobs was previously done with "exec". This was because it
allowed us to avoid copying too much container specification information
from the docker-compose/deployments files to the jobs files. However,
this was limiting:
- In order to run a job, the corresponding container had to be running.
This was particularly painful in Kubernetes, where containers are
crashing as long as migrations are not correctly run.
- Containers in which we need to run jobs needed to be present in the
docker-compose/deployments files. This is unnecessary, for example when
mysql is disabled, or in the case of the certbot container.
Now, we create dedicated jobs files, both for local and k8s deployment.
This introduces a little redundancy, but not too much. Note that
dependent containers are not listed in the docker-compose.jobs.yml file,
so an actual platform is still supposed to be running when we launch the
jobs.
This also introduces a subtle change: now, jobs go through the container
entrypoint prior to running. This is probably a good thing, as it will
avoid forgetting about incorrect environment variables.
In k8s, we find ourselves interacting way too much with the kubectl
utility. Parsing output from the CLI is a pain. So we need to switch to
the native kubernetes client library.
tls renewal and generation was failing in cron jobs because of "The
input device is not a TTY" errors. This is because the "-it" docker
option does not work when a tty is not available.
By de-duplicating the code between dev.py and local.py, we are able to
support more docker-compose run/up/stop options passed from tutor. To do
so, we had to disable some features, such as automatically mounting the
edx-platform repo when the TUTOR_EDX_PLATFORM_PATH environment variable
was defined.
Régis Behmo
Carlos Muniz
Overhang.IO
Kyle McCormick
Kyle McCormick
alex.soh
Régis Behmo