Enable O365 plugins and use ARM "condition" #32
Conversation
scripts/mysql_install_moodle.sh
Outdated
| # install Office 365 plugins | ||
| #if [ "$installOfficePlugins" = "True" ]; then |
There was a problem hiding this comment.
Looks like this change will always install the O365 plugins, they should be optional. Not all Moodle users need them.
There was a problem hiding this comment.
Fair enough, but my excuse is that we already install the ElasticSearch plugin (and dependencies) and the 3 VMs without any option. Shouldn't we make all these optional then? Note that that'll be a bigger code change.
Also note that the $installOfficePlugins env var was never defined at all, so I just couldn't uncomment the line (which is equivalent to no change at all).
There was a problem hiding this comment.
Yeah - that should be optional too :-)
scripts/postgres_install_moodle.sh
Outdated
| # install Office 365 plugins | ||
| #if [ "$installOfficePlugins" = "True" ]; then |
There was a problem hiding this comment.
ditto the comment above for the MySQL install.
As an aside we should look to remove as much duplication between the postgres and MySQL install scripts as possible. No need to block this PR on it, but something to bear in mind.
There was a problem hiding this comment.
Absolutely. I mentioned about this code duplication and wanted to do that properly, but that'll require some nontrivial refactoring of the shell scripts and modularization of that. I gave it some thoughts how to achieve that, but I realized that it's actually nontrivial. This is in line with our discussion with Anand about how applications are deployed through ARM. The current CustomScript extension-based one is quite crude and clunky, and yet there doesn't seem to be a good alternative yet.
There was a problem hiding this comment.
on ACS we used Cloud Init to great effect . I think it would serve our purpose here too. But again, we should make this separate from this PR which is valuable without that bigger change.
hosungsmsft
force-pushed
the
hs-condition-o365
branch
from
7a2435b
to
297227a
Compare
|
@rgardler Please take another look. Now O365 plugins and ElasticSearch (plugins & VMs) are both optional. |
hosungsmsft
force-pushed
the
hs-condition-o365
branch
2 times, most recently
from
abd073f
to
e09b5a1
Compare
hosungsmsft
force-pushed
the
hs-condition-o365
branch
from
e09b5a1
to
5502c5b
Compare
PR #32 changed the way we manage logic in the template to use the ARM conditional statements. Update the sample parameters file to match.
Office 365 Moodle plugins for Moodle v3.4 were recently released, so they can be installed by default. This change will install O365 plugins by default (no switch, though).
Also, this PR tries to simplify the conditional deployment by using ARM's "condition" property and removing the blank *config0.json files.
Test: Deployed the new templates on the default params only and confirmed they work, with O365 plugins installed on Moodle.