Skip to content
This repository has been archived by the owner on Aug 22, 2022. It is now read-only.

Update ansible vars and enable some features. #109

Merged
merged 1 commit into from
Jun 18, 2016

Conversation

mtyaka
Copy link
Member

@mtyaka mtyaka commented Jun 14, 2016

This adds some ansible vars to the yaml template mainly for documentation purposes (they are set to default values, so they should have no effect).

This commit also enables some extra feature flags, which are often useful and don't have any negative side effects (s3 grade downloads, third party auth...).

@mtyaka mtyaka force-pushed the mtyaka/update-default-vars branch 2 times, most recently from be29e90 to 94ca032 Compare June 15, 2016 09:01
@smarnach
Copy link
Contributor

@mtyaka I recently noticed that we should also set EDX_NOTES_API_VERSION to some reasonable value, and maybe we could include this change in this PR. (If you don't want that scope expansion, I can create a separate PR for that change.)

We currently don't set it at all, which means we always install the master version of edx_notes_api. This doesn't work when using the Dogwood version of configuration.

Unfortunately, edx_notes_api doesn't have the named-release/dogwood.3 tags or similar (in fact, it doesn't have any tags whatsoever), so using appserver.openedx_release would further restrict valid possible values for openedx_release. Do you have any good idea what to do about this?

@mtyaka
Copy link
Member Author

mtyaka commented Jun 16, 2016

@smarnach Thanks for bringing this up, I remember running into some problems because of this in the past. I guess the easiest thing to do would be to ask edX to create some named-release/* tags for edx_notes_api and then use appserver.openedx_release just like we do for other services (forums, xqueue, certs, etc).

@smarnach
Copy link
Contributor

@mtyaka Adding the tags is easy (in principle we could just do it ourselves). However, they would have to include the repo in their release script, so new releases get tagges automatically. I don't know what their process is, so this would require some coordination with edX. We should exclude this from this PR then, because otherwise we wouldn't get it merged in the current sprint.

This adds some ansible vars to the yaml template mainly for
documentation purposes (they are set to default values, so they should
have no effect).

This commit also enables some extra feature flags, which are often
useful and don't have any negative side effects (s3 grade downloads,
third party auth...).
@mtyaka mtyaka force-pushed the mtyaka/update-default-vars branch from 94ca032 to 7b4be3a Compare June 17, 2016 11:34
@smarnach
Copy link
Contributor

@mtyaka The instance you deployed is looking good, and I couldn't find any problems with it. The code is also looking good, so 👍.

@mtyaka mtyaka merged commit c25e3c3 into master Jun 18, 2016
@mtyaka mtyaka deleted the mtyaka/update-default-vars branch June 18, 2016 07:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants