feat: add picasso commands in simple way #2

MaferMazu · 2024-08-02T04:12:10Z

Description

This PR simply adds the following commands:

# Enable themes
tutor picasso enable-themes

# Enable private packages
tutor picasso enable-private-packages

# Run Tutor commands
tutor picasso run-extra-commands

For more information on how the plugin does it, follow the README.

How to test

Integration Tests

They are in the workflows.

Manually

Create a redwood tutor env
Install the plugin

pip install git+https://github.com/eduNEXT/tutor-contrib-picasso@mfmz/simple-picasso

Verify everything is right with the base commands.

# To enable the plugin
tutor plugins enable picasso

# Show help
tutor picasso -h

Add info in the config.yml, for example:

PICASSO_MANAGE_DPKG:
  name: eox-manage
  repo: [email protected]:eduNEXT/eox-manage.git
  version: v5.2.0
PICASSO_THEMES:
- name: endx-saas-themes
  repo: [email protected]:eduNEXT/ednx-saas-themes.git
  version: edunext/redwood.master
PICASSO_TENANT_DPKG:
  name: eox-tenant
  repo: [email protected]:eduNEXT/eox-tenant.git
  version: v11.7.0
PICASSO_EXTRA_COMMANDS:
- tutor plugins index add https://raw.githubusercontent.com/eduNEXT/tutor-plugin-indexes/main/
- tutor plugins install mfe mfe_extensions aspects
- tutor plugins enable mfe mfe_extensions aspects
- tutor picasso enable-themes
- tutor picasso enable-private-packages
- tutor config save

Run the commands available. If you are following the example, you only need to run:

tutor picasso run-extra-commands

Support

✅ Tested in Redwood, Quince, Palm, and Olive.

Warning

We recommend Quince >= v17.0.3 because add an important fix related to the enable-private-package command. Ref: overhangio/tutor#1016

Warning

If you want to use tutor plugin indexes, you need an Olive tutor version >= 15.3.0. (With this plugin the tutor plugin indexes is the only way to install other tutor plugins)

Important notes, and I want your feedback about it

My proposal here does not follow the release convention of the tutor plugin. To avoid major release when it is not needed. Another plugin with this system is Aspects: https://github.com/openedx/tutor-contrib-aspects. So, this plugin should be compatible with Palm and Quince.

Another proposal is only having those three commands. The syntax-validator and repository-validator don't add functionality to the build. If we want to still use it, we can use it in the GA or Jenkins pipeline through the scripts we have here: https://github.com/eduNEXT/dedalo-scripts/tree/main/jenkins.

Another important change is I proposed to change the structure of the input:

DISTRO_MY_PACKAGE_NAME_DPKG:
  index: git
  name: eox-package # directory name
  # ---- git package variables
  repo: eox-package # git repository name
  domain: github.com
  path: eduNEXT
  protocol: ssh
  # ---- end git package variables
  version: master
  private: true
  variables:
    development: {}
    production: {}

  PICASSO_<YOUR_PACKAGE_NAME>_DPKG:
    name: <your_package_name>
    repo: <your SSH URL for cloning the repo. e.g., [email protected]:yourorg/package.git>
    version: <your branch, tag o release for cloning. e.g., v5.2.0>

DISTRO_THEMES:
  - domain: github.com
    name: my-theme
    path: my-account
    protocol: ssh
    repo: my-openedx-theme
    version: release-compatible

  PICASSO_THEMES:
    - name: <your_theme_name>
      repo: <your SSH URL for cloning the repo. e.g., [email protected]:yourorg/theme.git>
      version: <your branch, tag o release for cloning. e.g., edunext/redwood.master>
    - name: <another_theme_name>
      repo: <your SSH URL for cloning the repo. e.g., [email protected]:yourorg/another_theme.git>
      version: <your branch, tag o release for cloning. e.g., edunext/redwood.blue>

README.rst

tutorpicasso/commands/enable_private_packages.py

tutorpicasso/commands/run_extra_commands.py

MaferMazu · 2024-08-20T23:11:12Z

I forget to mention this important proposal change:

I proposed to change the structure of the input:

DISTRO_MY_PACKAGE_NAME_DPKG:
  index: git
  name: eox-package # directory name
  # ---- git package variables
  repo: eox-package # git repository name
  domain: github.com
  path: eduNEXT
  protocol: ssh
  # ---- end git package variables
  version: master
  private: true
  variables:
    development: {}
    production: {}

  PICASSO_<YOUR_PACKAGE_NAME>_DPKG:
    name: <your_package_name>
    repo: <your SSH URL for cloning the repo. e.g., [email protected]:yourorg/package.git>
    version: <your branch, tag o release for cloning. e.g., v5.2.0>

DISTRO_THEMES:
  - domain: github.com
    name: my-theme
    path: my-account
    protocol: ssh
    repo: my-openedx-theme
    version: release-compatible

  PICASSO_THEMES:
    - name: <your_theme_name>
      repo: <your SSH URL for cloning the repo. e.g., [email protected]:yourorg/theme.git>
      version: <your branch, tag o release for cloning. e.g., edunext/redwood.master>
    - name: <another_theme_name>
      repo: <your SSH URL for cloning the repo. e.g., [email protected]:yourorg/another_theme.git>
      version: <your branch, tag o release for cloning. e.g., edunext/redwood.blue>

mariajgrimaldi

I left a few other suggestions for you to review and apply if you see fit. Thank you so much for addressing all my previous comments! Great work here :)

.github/workflows/integration_test.yml

tutorpicasso/commands/enable_private_packages.py

tutorpicasso/commands/enable_themes.py

tutorpicasso/commands/run_extra_commands.py

mariajgrimaldi · 2024-08-21T16:59:23Z

Thank you, @MaferMazu. Your proposal #2 (comment) looks way cleaner than what we currently use, so I have no objections.

Asespinel · 2024-08-22T04:12:08Z

Hello goodnight, I was following the instructions to test this plugin on the last redwood version (18.1.3) and when I try to enable the plugin using tutor plugins enable picasso I get the following error:

tutor plugins enable picasso
⚠️ Failed to enable plugin 'indigo': plugin 'indigo' is not installed.
⚠️ Failed to enable plugin 'mfe': plugin 'mfe' is not installed.
Error applying action: func=<function discover_package..load at 0x7f926ce698b0> contexts=['plugins']'
Traceback (most recent call last):
File "/home/andres/.tvm/v18.1.3@test-picasso/venv/bin/tutor", line 8, in
sys.exit(main())
File "/home/andres/.tvm/v18.1.3@test-picasso/overhangio-tutor-3143335/tutor/commands/cli.py", line 27, in main
cli() # pylint: disable=no-value-for-parameter
File "/home/andres/.tvm/v18.1.3@test-picasso/venv/lib/python3.8/site-packages/click/core.py", line 1157, in call
return self.main(*args, **kwargs)
File "/home/andres/.tvm/v18.1.3@test-picasso/venv/lib/python3.8/site-packages/click/core.py", line 1078, in main
rv = self.invoke(ctx)
File "/home/andres/.tvm/v18.1.3@test-picasso/venv/lib/python3.8/site-packages/click/core.py", line 1688, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/andres/.tvm/v18.1.3@test-picasso/venv/lib/python3.8/site-packages/click/core.py", line 1688, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/andres/.tvm/v18.1.3@test-picasso/venv/lib/python3.8/site-packages/click/core.py", line 1434, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/home/andres/.tvm/v18.1.3@test-picasso/venv/lib/python3.8/site-packages/click/core.py", line 783, in invoke
return __callback(*args, **kwargs)
File "/home/andres/.tvm/v18.1.3@test-picasso/venv/lib/python3.8/site-packages/click/decorators.py", line 33, in new_func
return f(get_current_context(), *args, **kwargs)
File "/home/andres/.tvm/v18.1.3@test-picasso/overhangio-tutor-3143335/tutor/commands/plugins.py", line 141, in enable
plugins.load(plugin)
File "/home/andres/.tvm/v18.1.3@test-picasso/overhangio-tutor-3143335/tutor/plugins/init.py", line 81, in load
hooks.Actions.PLUGIN_LOADED.do(name)
File "/home/andres/.tvm/v18.1.3@test-picasso/overhangio-tutor-3143335/tutor/core/hooks/actions.py", line 116, in do
self.do_from_context(None, *args, **kwargs)
File "/home/andres/.tvm/v18.1.3@test-picasso/overhangio-tutor-3143335/tutor/core/hooks/actions.py", line 135, in do_from_context
callback.do(
File "/home/andres/.tvm/v18.1.3@test-picasso/overhangio-tutor-3143335/tutor/core/hooks/actions.py", line 36, in do
self.func(*args, **kwargs)
File "/home/andres/.tvm/v18.1.3@test-picasso/overhangio-tutor-3143335/tutor/plugins/v1.py", line 78, in load
importlib.import_module(entrypoint.value)
File "/usr/lib/python3.8/importlib/init.py", line 127, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
File "", line 1014, in _gcd_import
File "", line 991, in _find_and_load
File "", line 975, in _find_and_load_unlocked
File "", line 671, in _load_unlocked
File "", line 848, in exec_module
File "", line 219, in _call_with_frames_removed
File "/home/andres/.tvm/v18.1.3@test-picasso/venv/lib/python3.8/site-packages/tutorpicasso/plugin.py", line 12, in
from tutorpicasso.commands.cli import picasso
File "/home/andres/.tvm/v18.1.3@test-picasso/venv/lib/python3.8/site-packages/tutorpicasso/commands/cli.py", line 9, in
from tutorpicasso.commands.run_extra_commands import run_extra_commands
File "/home/andres/.tvm/v18.1.3@test-picasso/venv/lib/python3.8/site-packages/tutorpicasso/commands/run_extra_commands.py", line 127, in
def create_regex_from_list(special_chars: List[str]) -> re.Pattern[str]:
TypeError: 'type' object is not subscriptable

However, when I do tutor plugins list I do see the plugin installed.

mariajgrimaldi

I'll leave my approval once these last suggestions are addressed. Thank you!

mariajgrimaldi · 2024-08-22T14:58:27Z

tutorpicasso/commands/enable_private_packages.py

+    context = click.get_current_context().obj
+    tutor_root = context.root
+    tutor_conf = tutor_config.load(tutor_root)
+
+    tutor_version_obj = Version(tutor_version)
+    # Define Quince version as the method for installing private packages changes from this version
+    quince_version_obj = Version("v17.0.0")
+
+    # Use these specific paths as required by Tutor < Quince
+    private_requirements_root = f"{tutor_root}/env/build/openedx/requirements"
+    packages = get_picasso_packages(tutor_conf)
+
+    # Create necessary directory if it doesn't exist
+    if not os.path.exists(private_requirements_root):
+        os.makedirs(private_requirements_root)
+
+    for package, info in packages.items():
+        if not {"name", "repo", "version"}.issubset(info):
+            raise click.ClickException(
+                f"{package} is missing one of the required keys: 'name', 'repo', 'version'"
+            )
+
+        if os.path.isdir(f'{private_requirements_root}/{info["name"]}'):
+            subprocess.call(
+                ["rm", "-rf", f'{private_requirements_root}/{info["name"]}']
+            )
+
+        subprocess.call(
+            ["git", "clone", "-b", info["version"], info["repo"]],
+            cwd=private_requirements_root,
+        )
+
+        if tutor_version_obj < quince_version_obj:
+            private_requirements_txt = f"{private_requirements_root}/private.txt"
+            _enable_private_packages_before_quince(info, private_requirements_txt)
+        else:
+            _enable_private_packages(info, private_requirements_root)


What do you think about doing something like this: 7c6ccc3.

[nit] We're using both packages and requirements in naming. Could we decide whether to use packages or requirements?

Thanks a lot. I applied 7c6ccc3 and also refactored a little bit the names of the functions.

Regarding naming, I propose that in the context of Picasso, we use the name package (to read the package information and enable packages, but for the context of Tutor, we use the name requirements); I don't know if there is a good place to put it.

I maintained the package command to avoid being so far to tutor-contrib-edunext-distro; but later, we can question the use of package instead of requirements.

tutorpicasso/commands/run_extra_commands.py

mariajgrimaldi · 2024-08-22T15:29:55Z

tutorpicasso/commands/run_extra_commands.py

+                "Take a look at the official Tutor commands: "
+                "https://docs.tutor.edly.io/reference/cli/index.html"
+            )
+            return error_message


question: if multiple commands have errors, will this only return the first it encounters? what if returns them all 87e8503?

Thanks for this. I applied 87e8503 and applied a little fix. You can check and verify if everything is okay.

bra-i-am

@MaferMazu, thanks a lot for this PR. I only pointed out some minor things

.github/workflows/integration_test.yml

README.rst

Co-authored-by: Brayan Cerón <[email protected]>

MaferMazu · 2024-08-23T21:48:16Z

@Asespinel, I couldn't replicate the error.

Step by step in local (It works):

tvm project init picasso_v1813 v18.1.3
cd picasso_v1813/
source .tvm/bin/activate 
pip install git+https://github.com/eduNEXT/tutor-contrib-picasso@mfmz/simple-picasso
tutor plugins enable picasso

Also, in the integration test, we test this plugin in Tutor == 18.1.3 in the <19.0.0 integration test. For more information: https://github.com/eduNEXT/tutor-contrib-picasso/actions/runs/10532688714/job/29187320715 (Or you can check the <19.0.0 integration test)

Please let me know if the problem persists.

…version

mariajgrimaldi

Thank you so much for addressing all my comments. LGTM!

MaferMazu · 2024-08-27T01:14:10Z

@Asespinel I already fix the problem. I couldn't replicate it before because I was using Python 3.10, and the issue was for version < 3.9. It was because I couldn't call re.Pattern[str]. Now, I am using Pattern from typing. You should be able to try it now.

bra-i-am

LGTM! 👯

feat: add picasso commands in simple way

3882d9b

MaferMazu force-pushed the mfmz/simple-picasso branch from 6e9c13c to c544e98 Compare August 2, 2024 04:13

MaferMazu added 2 commits August 1, 2024 23:21

test: add workflows and fix tests

77e973a

docs: add information in the readme

dea0046

MaferMazu force-pushed the mfmz/simple-picasso branch 2 times, most recently from b0c99b2 to dceae00 Compare August 2, 2024 17:56

test: fix tests

c740aa4

MaferMazu force-pushed the mfmz/simple-picasso branch from dceae00 to c740aa4 Compare August 2, 2024 17:58

MaferMazu marked this pull request as ready for review August 2, 2024 18:17

MaferMazu requested review from magajh, Alec4r, BryanttV, mariajgrimaldi, dcoa, luisfelipec95, Asespinel, bra-i-am and jignaciopm August 2, 2024 18:17

MaferMazu added 2 commits August 5, 2024 10:15

fix: add importlib_resources for palm support

a4c769b

fix: add commands in picasso installation

9e035ec

mariajgrimaldi requested changes Aug 5, 2024

View reviewed changes

MaferMazu force-pushed the mfmz/simple-picasso branch 2 times, most recently from 6d5ab6f to 4d3bf9f Compare August 16, 2024 17:06

feat: add backward compatibility

f977bd3

MaferMazu force-pushed the mfmz/simple-picasso branch from 4d3bf9f to f977bd3 Compare August 16, 2024 17:25

fix: fix tests

1a5d0b5

MaferMazu force-pushed the mfmz/simple-picasso branch from 8d842b4 to 924b175 Compare August 16, 2024 18:32

fix: add condition before MOUNTED_DIRECTORIES

1cd0557

MaferMazu force-pushed the mfmz/simple-picasso branch 2 times, most recently from 83239a9 to 6b384ad Compare August 16, 2024 19:03

MaferMazu force-pushed the mfmz/simple-picasso branch from d07a938 to a078a8b Compare August 20, 2024 23:07

test: add again the test-types

3c8dc2e

MaferMazu requested a review from mariajgrimaldi August 20, 2024 23:41

mariajgrimaldi requested changes Aug 21, 2024

View reviewed changes

fix: apply feedback about quick checks and separate methods

e1ad584

MaferMazu force-pushed the mfmz/simple-picasso branch from 1e326cf to e1ad584 Compare August 22, 2024 02:25

mariajgrimaldi requested changes Aug 22, 2024

View reviewed changes

bra-i-am requested changes Aug 22, 2024

View reviewed changes

docs: update the readme format

0203763

Co-authored-by: Brayan Cerón <[email protected]>

MaferMazu force-pushed the mfmz/simple-picasso branch from 1540bd4 to 0203763 Compare August 23, 2024 21:34

chore: improve integration tests

18f983e

MaferMazu requested a review from bra-i-am August 26, 2024 16:38

mariajgrimaldi and others added 2 commits August 26, 2024 15:52

refactor: use handler to manage requirement tasks depending on tutor …

c8eefa0

…version

refactor: improve the naming in enable_private_packages

5d8dc5b

MaferMazu force-pushed the mfmz/simple-picasso branch from 9cf3ec5 to 5d8dc5b Compare August 26, 2024 21:24

MaferMazu and others added 3 commits August 26, 2024 16:35

docs: improve code docs

821c5fc

refactor: return all command error messages instead

018b455

fix: check the commands inside the loop and format

f03da32

MaferMazu requested a review from mariajgrimaldi August 26, 2024 21:47

MaferMazu added 2 commits August 26, 2024 16:56

docs: add documentation about the type ignore

26fb79a

fix: use typing Pattern instead of regex one

035f49d

mariajgrimaldi approved these changes Aug 27, 2024

View reviewed changes

bra-i-am approved these changes Aug 27, 2024

View reviewed changes

MaferMazu merged commit bf51ed8 into main Aug 27, 2024
7 checks passed

MaferMazu mentioned this pull request Aug 28, 2024

feat: add picasso commands #1

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add picasso commands in simple way #2

feat: add picasso commands in simple way #2

MaferMazu commented Aug 2, 2024 •

edited

Loading

MaferMazu commented Aug 20, 2024

mariajgrimaldi left a comment •

edited

Loading

mariajgrimaldi commented Aug 21, 2024

Asespinel commented Aug 22, 2024 •

edited

Loading

mariajgrimaldi left a comment

mariajgrimaldi Aug 22, 2024

mariajgrimaldi Aug 22, 2024

MaferMazu Aug 26, 2024

mariajgrimaldi Aug 22, 2024

MaferMazu Aug 26, 2024

bra-i-am left a comment •

edited

Loading

MaferMazu commented Aug 23, 2024

mariajgrimaldi left a comment

MaferMazu commented Aug 27, 2024

bra-i-am left a comment

feat: add picasso commands in simple way #2

feat: add picasso commands in simple way #2

Conversation

MaferMazu commented Aug 2, 2024 • edited Loading

Description

How to test

Integration Tests

Manually

Support

Important notes, and I want your feedback about it

MaferMazu commented Aug 20, 2024

mariajgrimaldi left a comment • edited Loading

Choose a reason for hiding this comment

mariajgrimaldi commented Aug 21, 2024

Asespinel commented Aug 22, 2024 • edited Loading

mariajgrimaldi left a comment

Choose a reason for hiding this comment

mariajgrimaldi Aug 22, 2024

Choose a reason for hiding this comment

mariajgrimaldi Aug 22, 2024

Choose a reason for hiding this comment

MaferMazu Aug 26, 2024

Choose a reason for hiding this comment

mariajgrimaldi Aug 22, 2024

Choose a reason for hiding this comment

MaferMazu Aug 26, 2024

Choose a reason for hiding this comment

bra-i-am left a comment • edited Loading

Choose a reason for hiding this comment

MaferMazu commented Aug 23, 2024

mariajgrimaldi left a comment

Choose a reason for hiding this comment

MaferMazu commented Aug 27, 2024

bra-i-am left a comment

Choose a reason for hiding this comment

MaferMazu commented Aug 2, 2024 •

edited

Loading

mariajgrimaldi left a comment •

edited

Loading

Asespinel commented Aug 22, 2024 •

edited

Loading

bra-i-am left a comment •

edited

Loading