Paver #2065
Paver #2065
Conversation
|
I love the commit! Thanks @dglance and @singingwolfboy. We had a few tries in the past to move to something like Paver, but were never completed. What is the plan now, deprecate the rake commands? Is there a way that we can make more explicit? If we deprecate rake, how does it affect the deployment on edx.org? We need to have devops look into it too, @jarv, @e0d, @feanil comments? |
|
Please keep me and @wedaly in the loop before merging this in. It has implications on the jenkins setups, especially if we're looking to deprecate rake. |
|
Recenlty was merge https://github.com/edx/edx-platform/pull/1957 to consider. |
| """ | ||
| runs lms without running prereqs | ||
| """ | ||
| sh("python manage.py lms runserver --traceback --settings=dev --pythonpath=. %s" % default_options['lms']) |
There was a problem hiding this comment.
Is this fast_lms() different from running the lms task with "dev" as the environment? Might not be needed.
There was a problem hiding this comment.
Originally I did a 1-for-1 replacement of the rake functions but as we are doing a tidy-up at the same time as replacing rake, it is worth thinking about what is actually needed or not needed. In the current scheme of things, fast_lms isn't different from the normal running of lms
|
I am still here btw :) I was originally going to do this in two parts: [a] submit paver it seems that people want [a] + [b] in one go. So @singingwolfboy - what do you want me to do? I have the code for [b] mostly done - do you want me to resubmit a PR with both [a] and [b]? Do you need tests and if so, do they just need to check that the coverage of functions deprecated or the actual functionality of paver? |
|
@dglance Ack, sorry it took me so long to follow-up on this. Yes, [a] and [b] together would be great -- we don't want to have multiple ways to do the same thing. Tests would be awesome, but since these paver commands are going to be part of our daily routine, we'll all be manually testing them every day, so I don't think automated tests are necessary. |
| except KeyboardInterrupt: | ||
| pass | ||
| except: | ||
| pass |
There was a problem hiding this comment.
I'm curious about this pattern of using subprocess. It seems elaborate. Why do we need all of this machinery? Do we have unusual requirements that most Paver users don't? What is the typical Paver solution to the problem this is solving? If we do decide to keep it, at least we can encapsulate it somehow so we don't have to repeat it.
There was a problem hiding this comment.
This is for running Sass, Coffeescript etc with the watch flag. This creates children and they don't get cleaned up by sending a signal to the parent (for some reason) - at least not on MacOS. The other aspect is handling the ctl-c interrupt so that it is handled properly and no errors are not printed on the console.
Resubmit of rake replacement of assets, django, docs and prereqs Deprecated rake functions issue a warning and then call paver replacements
Changes on feedback from @cpennington and @singingwolfboy
Based on feedback from @singingwolfboy and @cpennington
* [MCKIN-27241] Merge user progress when target is enrolled but has no progress (openedx#2065) During user progress migration, targets already enrolled in the course are now acceptable if they have no progress in that course already. In case progress already exists, the migration to that user will be skipped. * MCKON-27241 Fixed function name * Skip test if not lms Co-authored-by: Paulo Viadanna <paulo@opencraft.com>
…progress (openedx#2065) During user progress migration, targets already enrolled in the course are now acceptable if they have no progress in that course already. In case progress already exists, the migration to that user will be skipped.
Rebased and squashed version of #1687