search

eduNEXT / tutor-contrib-edunext-distro

A tool to facilitate the customization of an Openedx instance, adding commands and settings to have an easy-to-use and a ready-to-deploy in local or in development openedx distribution.
2 stars 1 forks source link

fix: add condition to validate if element "variable" exists #44

Closed luisfelipec95 closed 1 year ago

luisfelipec95 commented 1 year ago

Description

This PR fixes the following error: If a package is added without variables this will raise the following error 190263652-8c1c3d90-b952-4e7d-80ea-8a96d631264d

How to test

pip install git+https://github.com/eduNEXT/tutor-contrib-edunext-distro.git@DS-481 tutor plugins enable distro

Example: "EOX_THEMING_DPKG": { "index": "git", "name": "eox-theming", "repo": "eox-theming", "version": "v5.0.0", "domain": "github.com", "protocol": "https", "path": "eduNEXT", "EOX_THEMING_CONFIG_SOURCES":[ "from_eox_tenant_microsite_v2", "from_django_settings" ], "private": False, },

tutor config save

dcoa commented 1 year ago

Looks good, I just wondering if could be the case that I define just one type of variable, for example, production variables but not dev variables.

JuanDavidBuitrago commented 1 year ago

It's possible, but it works also if you use a variable, I tested this case too.

luisfelipec95 commented 1 year ago

@dcoa do you have another comment?

JuanDavidBuitrago commented 1 year ago

I tested it and I realized 3 things:

* There is a typo that makes any variable charged in the environment.

Thanks @dcoa, I had not noticed this error 

* I test the use case of my question and fail.

I test changing the typo mistake and fail as you say Define dev variables but not production: image

I left a comment explaining how we can change the line to support that case.

* Can we delete the empty definitions in plugin.py?

I think it is a good idea to use this PR to delete the empty definitions, could you do this? @luisfelipec95

luisfelipec95 commented 1 year ago

changes made

dcoa commented 1 year ago

@ hi, @luisfelipec95 please don't forget to update plugin.py as well.

MaferMazu commented 1 year ago

@luisfelipec95, @dcoa proposes to edit the plugin.py file to delete the empty variables, for example, this https://github.com/eduNEXT/tutor-contrib-edunext-distro/blob/master/tutordistro/plugin.py#L82. I think it is a good idea. Can you do it? The rest looks good to me :D

luisfelipec95 commented 1 year ago

@luisfelipec95, @dcoa proposes to edit the plugin.py file to delete the empty variables, for example, this https://github.com/eduNEXT/tutor-contrib-edunext-distro/blob/master/tutordistro/plugin.py#L82. I think it is a good idea. Can you do it? The rest looks good to me :D

Done, thanks