-
Notifications
You must be signed in to change notification settings - Fork 1
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
(DS-536) refactor!: remove distro defaults settings to add from strain/conf file #46
Conversation
d69ae17
to
89bbe7a
Compare
@JuanDavidBuitrago this looks good for me, just a comment, for next PRs, please split the CI and new feature in differents PRs and I have a question, are there other strains using olive or olmo? Also, what will happend if my config.yml doesn't have DISTRO_EXTRA_MIDDLEWARES or the others old default variables? will it work? |
e3c0fff
to
5c1ca96
Compare
@Alec4r I split the CI commit, is in this PR.
I fixed all problems when the defaults variables aren't defined, so if a user doesn't need to use all of them, Distro continue working normally Can you check again?, please |
tutordistro/patches/openedx-dockerfile-post-python-requirements
Outdated
Show resolved
Hide resolved
We should update the documentation to remove all references to default values. |
5c1ca96
to
f73e613
Compare
21a2654
to
9689c0b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late review and many comments; this PR is a big breaking change, and I believe it is so important that the users of Distro understand what is happening.
I would like you to change a little bit the description of what the Distro is and what this plugin does after this PR and try to adapt the Readme to be easy to know what the user can do; for that reason, I let you several comments around to drop the default explanation and go straight to how to do the things they need to do.
I have a little question, when we merge this, are we going to have an "Olmo" version of the Distro with this plugin in v16? When we migrate a palm, are we planning to stay in v16?
The rest looks good.
I don't think the changes that were made drastically change the description of the Distro. It's just that now we're not going to have Django plugins and Themes by default, now users are free to use whatever they want in their environment. Regarding the version, this continues to lead to the nomenclature according to the version of Tutor, as it has been working up to now. Remember that in Nutmeg (v14) we went from a v3.2.0 Tag to v14.2.0 just to be consistent with Tutor versions. |
@MaferMazu Let me know if anything else bothers you, so I can clear it up. |
bd29188
to
7bec999
Compare
Yes, I really don't understand.
And If I see Distro as distribution, yes. But this plugin with these changes won't be the opinionated openedx distribution, maybe a tool we use to make easier our work (we are going to have our tools), but not the opinionated distribution. (Probably is a good idea to add some information more accurate about what this plugin really will do)
Maybe I need to understand better what we want to do with this plugin, for that reason, I am not feeling comfortable giving a review. |
Hi @Alec4r, could you give me a hand here? Please |
We have already talk about this, but I could explain again: Distro is an opinioned openedx distribution.: Yes, The most important part is "is an opinioned openedx distribution", distro is just a number of requirements, settings, xblocks, that edunext considerate are the best, but is just an opinion, so we will move that to an one pager, documentation and strain, due to it's just an opinion, it's similar to NELP opinion, Pearson Opinion, are just opinions. with some custom stuff to have an easy-to-use and a ready to deploy in local or in development openedx distribution: Second part, how to use this, ok, the answer for this part is "Distro plugin/Tutor Stack Plugin" that help us to load opinions (Strains) easier, but it's just another way to do it, because you can apply NELP opinion without use a plugin, you could use inline plugins, and you can configure each theme or private requirements without need "Distro plugin/Tutor Stack Plugin", but maybe with this plugin is easier than replicate each step. Why on this version and not wait for palm?, because we need to test now, and be safe all is working while we can change between versions and not in palm while we are in the middle of a big migration. If you want to see this like "Distro Plugin v2.0" it's ok, this is a big rework of our old plugin. And of course this will need new documentation, I totally agree with you. Now, this is the reason why we used to have difference version and we couldn't use just "Tutor Semantic Version", because our break change doesn't depends of tutor version, this is a tool more than a tutor plugin, we are just using tutor technologies for make easier our work. |
@JuanDavidBuitrago did you already do some test using picasso? could you give me the link or number of the build? |
Yes @Alec4r, I did the build with this olmo strain and the job in Picasso is #121. This is the Docker Image Let me know if you need anything else. |
Description
This PR remove
Distro defaults settings
to have a Tutor plugins flexible for different users, The principal idea is that people use Distro but with the packages that want to use, making the settings in a config/strain file.For example, our Olmo master version is set in strain.yml file and contain
Django plugins
/Themes
that use our SAAS.Here are the settings that were removed and added in strain file: https://github.com/eduNEXT/ednx-strains/pull/46/files
How to test
Using stack builder in your local to build the image
stack strain create
commandstack strain local configure -s install_plugins -s enable_plugins -s save_config -s extra_commands
run stack strain local build openedx --no-cache
Using Picasso to build the image
STRAIN_REPOSITORY_BRANCH
setjdb/add_default_settings_to_distro
, it's the branch where are the settings to DistroSTRAIN_PATH
setolmo/master
, it's the strain that has the Distro settingsBuild
button