From 30aecc318128e7825787d454597910a5ab3d4c17 Mon Sep 17 00:00:00 2001 From: Brayan Ceron Date: Mon, 8 Jul 2024 11:44:19 -0500 Subject: [PATCH] refactor: address the feedback given * refactor: improve & update method of compiling themes * refactor: enhance extra command logs * docs: Clarify & complement changelog entry --- CHANGELOG.md | 6 ++++-- README.md | 6 ++++-- .../distro/extra_commands/infrastructure/tutor_commands.py | 6 ++++-- tutordistro/patches/openedx-dockerfile-pre-assets | 4 ++-- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7152c1..89534e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,9 +16,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 #### ⚠ BREAKING CHANGES -* Add support to Redwood ([885e0b9](https://github.com/eduNEXT/tutor-contrib-edunext-distro/commit/885e0b955722e4d86f89547764537ee74f707cf9)); the main change +* [Feature] Add support to Redwood ([885e0b9](https://github.com/eduNEXT/tutor-contrib-edunext-distro/commit/885e0b955722e4d86f89547764537ee74f707cf9)) -* Fix when 'tutor distro enable-themes' was executed before, a new execution hung when it tried to overwrite the existing theme folder. The process requires confirmation ('yes') to proceed with the overwrite, but it did not receive this input automatically, leading to a freeze ([1924007](https://github.com/eduNEXT/tutor-contrib-edunext-distro/commit/1924007d121b46c9dc5949810f1e7f89fdf0fad2)) +* [Bugfix] Adapt the inline Tutor plugin `openedx-dockerfile-pre-assets` to stop using the deprecated command [openedx-assets](https://github.com/overhangio/tutor/blob/master/CHANGELOG.md#v1800-2024-06-19:~:text=%F0%9F%92%A5%5BFeature%5D%20The%20openedx,or%20cms%20container%3A) for setting the pre-assets of the custom themes added with Distro ([885e0b9](https://github.com/eduNEXT/tutor-contrib-edunext-distro/commit/885e0b955722e4d86f89547764537ee74f707cf9#diff-d32c8a1ee8b6076c6fb3375498a9d455d41cad3104464e9b1e3900fd4265160a)) + +* [Bugfix] 'tutor distro enable-themes' command would hang when trying to overwrite an existing theme folder due to not being able to read the confirmation input ('yes') to perform the overwrite ([1924007](https://github.com/eduNEXT/tutor-contrib-edunext-distro/commit/1924007d121b46c9dc5949810f1e7f89fdf0fad2)). ## v17.2.0 - 2024-04-05 diff --git a/README.md b/README.md index 075e68d..03a0d98 100644 --- a/README.md +++ b/README.md @@ -237,14 +237,16 @@ tutor distro enable-themes > tutor images build openedx-dev > tutor dev do init > tutor dev start -> tutor dev run lms openedx-assets themes --theme-dirs [THEME_DIRS] --themes [THEME_NAMES] +> tutor dev exec lms bash +> npm run compile-saas -- theme_dirs [THEME_DIRS] themes [THEME_NAMES] > ``` > > or > > ```bash > tutor dev launch -> tutor dev run lms openedx-assets themes --theme-dirs [THEME_DIRS] --themes [THEME_NAMES] +> tutor dev exec lms bash +> npm run compile-saas -- theme_dirs [THEME_DIRS] themes [THEME_NAMES] > ``` # Commands diff --git a/tutordistro/distro/extra_commands/infrastructure/tutor_commands.py b/tutordistro/distro/extra_commands/infrastructure/tutor_commands.py index 8c92dba..f998695 100644 --- a/tutordistro/distro/extra_commands/infrastructure/tutor_commands.py +++ b/tutordistro/distro/extra_commands/infrastructure/tutor_commands.py @@ -69,8 +69,6 @@ def run_command(self, command: str): command (str): Tutor command. """ try: - print(f'Running "{command}"') - with subprocess.Popen( command, shell=True, @@ -81,6 +79,7 @@ def run_command(self, command: str): text=True, ) as process: + # It is sent a 'y' to say 'yes' on overriding the existing folders stdout, stderr = process.communicate(input="y") if process.returncode != 0 or "error" in stderr.lower(): @@ -88,5 +87,8 @@ def run_command(self, command: str): process.returncode, command, output=stdout, stderr=stderr ) + # This print is left on purpose to show the command output + print(stdout) + except subprocess.CalledProcessError as error: raise CommandError(f"\n{error.stderr}") from error diff --git a/tutordistro/patches/openedx-dockerfile-pre-assets b/tutordistro/patches/openedx-dockerfile-pre-assets index a315fc7..edbea11 100644 --- a/tutordistro/patches/openedx-dockerfile-pre-assets +++ b/tutordistro/patches/openedx-dockerfile-pre-assets @@ -8,8 +8,8 @@ COPY --chown=app:app ./themes/ {{ DISTRO_THEMES_ROOT }} {% endif %} {% if DISTRO_THEME_DIRS is defined and DISTRO_THEMES_NAME is defined %} RUN npm run compile-sass -- \ - --theme-dir {{ DISTRO_THEME_DIRS | join(' --theme-dir ') }} \ - --theme {{ DISTRO_THEMES_NAME | join(' --theme ') }} \ + theme_dirs {{ DISTRO_THEME_DIRS | join(' ') }} \ + themes {{ DISTRO_THEMES_NAME | join(' ') }} \ && ./manage.py lms collectstatic --noinput --settings=tutor.assets \ && rdfind -makesymlinks true -followsymlinks true /openedx/staticfiles/ {% endif %}