overhangio / tutor-mfe

This plugin makes it possible to easily add micro frontend (MFE) applications on top of an Open edX platform that runs with Tutor.
GNU Affero General Public License v3.0
22 stars 95 forks source link

feat: introduces MFE_DOCKER_IMAGE_DEV_PREFIX #216

Closed rediris closed 4 months ago

rediris commented 5 months ago

Introduces a MFE_DOCKER_IMAGE_DEV_PREFIX setting to customize dev MFE image docker repos. See discussion below for the benefits.

rediris commented 5 months ago

@regisb thoughts?

regisb commented 5 months ago

I'm not sure I understand. What problem exactly is this PR solving? Please describe your use case precisely, and how this change improves your workflow.

rediris commented 5 months ago

Yes, this probably deserves a bit more explanation than what I provided above.

Scenario 1: when customizing existing MFEs Using a custom docker repo for MFE development images is currently impossible because the overhangio images are hard-coded into the plugin. While it is possible to override the tutor local mode production MFE image by using MFE_DOCKER_IMAGE in config.yml, this does not apply to the individual MFE images when running tutor in dev mode. This creates a bit of a development bottleneck, as the dev is forced to manually build/rebuild the development images instead of expediently pulling them from the custom docker repo. Additionally, the current process seems to necessitate using --no-cache and/or --no-registry-cache flags to prevent automatically pulling the default overhangio images.

Scenario 2: when creating new MFEs The hard-coded overhangio image references create a similar problem when creating new MFEs. Let's say I have a custom MFE called frontend-app-customizeme - the resulting docker image path will look like docker.io/overhangio/openedx-customizeme-dev:17.0.1 (based on what I'm seeing here: https://github.com/overhangio/tutor-mfe/blob/master/tutormfe/plugin.py#L154-L155).

Let me know if any of that makes sense...

regisb commented 5 months ago

OK I understand. For once, the "right" fix for this is to introduce a new setting: MFE_DOCKER_IMAGE_DEV_PREFIX="{{ DOCKER_REGISTRY }}overhangio/openedx-". Can you please change your PR to follow this approach? Please remember to include a changelog entry: https://docs.tutor.overhang.io/tutor.html#contributing

rediris commented 5 months ago

OK I understand. For once, the "right" fix for this is to introduce a new setting: MFE_DOCKER_IMAGE_DEV_PREFIX="{{ DOCKER_REGISTRY }}overhangio/openedx-". Can you please change your PR to follow this approach? Please remember to include a changelog entry: https://docs.tutor.overhang.io/tutor.html#contributing

Will do, thank you very much!

rediris commented 5 months ago

@regisb all set, let me know what you think!

rediris commented 5 months ago

Oops, forgot the changelog...

hinakhadim commented 4 months ago

I have tested it. LGTM!