Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] fix: add script to install node_modules in mounted mfes #232

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions tutormfe/patches/dev-docker-compose-jobs-services
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{%- for app_name, app in iter_mfes() %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this patch needed?

{%- set mounts = iter_mounts(MOUNTS, app_name)|list %}
{%- if mounts %}
{{ app_name }}-job:
image: "{{ MFE_DOCKER_IMAGE_DEV_PREFIX }}-{{ app_name }}-dev:{{ MFE_VERSION }}"
volumes:
{%- for mount in mounts %}
- {{ mount }}
{%- endfor %}
{%- endif %}
{%- endfor %}
25 changes: 25 additions & 0 deletions tutormfe/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import functools
import os
import re
import typing as t
from glob import glob

Expand Down Expand Up @@ -304,3 +305,27 @@ def _check_mfe_host(config: Config) -> None:
f'Warning: MFE_HOST="{mfe_host}" is not a subdomain of LMS_HOST="{lms_host}". '
"This configuration is not typically recommended and may lead to unexpected behavior."
)


@tutor_hooks.Actions.CONFIG_LOADED.add()
def _run_jobs_in_mounted_mfes(config: Config) -> None:
mounts = get_typed(config, "MOUNTS", list, [])

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can return here if no mounts exist in the config.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

pattern = re.compile(r'frontend-app-(\w+)')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use the REPO_PREFIX variable defined above here instead of a hardcoded string.

mounted_mfes = [match.group(1) for mount in mounts if (match := pattern.search(mount))]

mfe_task_file = os.path.join(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mfe_npm_install could be a better name as this action is related only to npm install.

str(
importlib_resources.files("tutormfe")
/ "templates"
/ "mfe"
/ "tasks"
/ "mfe"
/ "npm-install.sh"
)
)

for mounted_mfe in mounted_mfes:
with tutor_hooks.Contexts.app("mfe").enter():
with open(mfe_task_file, encoding='utf-8') as fi:
tutor_hooks.Filters.CLI_DO_INIT_TASKS.add_item((mounted_mfe , fi.read()))
36 changes: 36 additions & 0 deletions tutormfe/templates/mfe/tasks/mfe/npm-install.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#!/bin/bash

# Paths for the fingerprint and package-lock.json
FINGERPRINT_FILE="node_modules/.fingerprint"
PACKAGE_LOCK="package-lock.json"

# Function to generate the SHA1 hash of the package-lock.json file
generate_fingerprint() {
sha1sum "$PACKAGE_LOCK" | awk '{print $1}'
}

# Check if node_modules exists and fingerprint is valid
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment can be broken down and mentioned against the appropriate if-elif-else block.

if [ ! -d "node_modules" ]; then
echo "⚠️ node_modules not found! Installing dependencies..."
npm clean-install

elif [ ! -f "$FINGERPRINT_FILE" ]; then
echo "⚠️ Fingerprint file not found! Re-installing dependencies..."
npm clean-install

else
CURRENT_FINGERPRINT=$(generate_fingerprint)
STORED_FINGERPRINT=$(cat "$FINGERPRINT_FILE")

if [ "$CURRENT_FINGERPRINT" != "$STORED_FINGERPRINT" ]; then
echo "⚠️ package-lock.json changed! Re-installing dependencies..."
npm clean-install
else
echo "✅ Dependencies are up to date."
exit 0
fi
fi

# Store the new fingerprint after installation
generate_fingerprint > "$FINGERPRINT_FILE"
echo "✅ Dependencies installed and fingerprint updated."
Loading