Skip to content
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

Hot fix for failing video downloads on OSX. #4115

Conversation

cpauya
Copy link
Contributor

@cpauya cpauya commented Jul 17, 2015

Hi @aronasorman, this PR does the following:

As per learningequality/ka-lite-installers#100 - PyRun doesn't include the multiprocessing module by default. So we check if call_command() is being run inside PyRun and call another method that uses the subprocess module.

If not running on Pyrun, we resume with current functionality that calls the CommandProcess class which uses the multiprocessing module.

To test this with Pyrun

  1. Checkout the installers repo on another directory, then run setup.sh of the OSX installers repo.
  2. Press Ctrl+C or cancel when it's already downloading the "ka-lite" repo, we only need the downloaded PyRun package.
  3. Checkout this PR's branch.
  4. Run "<installers-working-dir>/installers/osx/temp/pyrun-2.7/bin/pip" install . to install the ka-lite-static module.
  5. Run "<working-dir>/installers/osx/temp/pyrun-2.7/bin/pip" freeze - to see double-check that ka-lite-static is installed.
  6. Run launchctl setenv KALITE_PYTHON "<bin/pyrun full path>". Example: launchctl setenv KALITE_PYTHON "/Users/cyril/w/fle/ka-lite/installers/osx/temp/pyrun-2.7/bin/pyrun"
  7. Restart the terminal.
  8. Check that we have a KALITE_PYTHON env var with env | grep KA.
  9. You can now run bin/kalite manage setup command and test the device registration, video downloads, language downloads, and etc to see if they work.

To test with the OSX default Python, just unset the KALITE_PYTHON with launchctl unsetenv KALITE_PYTHON and restart the server.

Tips

  1. Make an alias of the 'pyrun' and 'pip' to ease typing.
  2. Set DEBUG=True in your local_settings.py for easier tracing.

Other fix
This also sets the default value for the DO_NOT_RELOAD_CONTENT_CACHE_AT_STARTUP = False at kalite/settings/__init__.py to fix crashes when settings.DO_NOT_RELOAD_CONTENT_CACHE_AT_STARTUP is used in other Python codes as per #4114. I had to do this to continue testing the fixes above.

…the `multiprocessing` module.

Set default value for the `DO_NOT_RELOAD_CONTENT_CACHE_AT_STARTUP = False` at `kalite/settings/__init__.py` to fix crashes when `settings.DO_NOT_RELOAD_CONTENT_CACHE_AT_STARTUP` is used in other Python codes.
@cpauya cpauya added the Mac label Jul 17, 2015
@cpauya cpauya added this to the 0.14.x bugfixes milestone Jul 17, 2015
@cpauya cpauya added the has PR label Jul 17, 2015
@@ -46,6 +46,8 @@ def package_selected(package_name):
RemovedInKALite_v015_Warning
)

DO_NOT_RELOAD_CONTENT_CACHE_AT_STARTUP = getattr(local_settings, "DO_NOT_RELOAD_CONTENT_CACHE_AT_STARTUP", False)

This comment was marked as spam.

@benjaoming benjaoming mentioned this pull request Jul 17, 2015
5 tasks
@cpauya
Copy link
Contributor Author

cpauya commented Jul 20, 2015

Hi @benjaoming - yes we are still using local_settings.py on OSX. I tried using bin/kalite start --settings kalite.my_settings but couldn't remember why I can't make it work.

But we are leaving local_settings.py as soon as 0.14.x is released - had to deal with more important tasks. :)

I saw your merged PR already and will use that. Thanks.

@mrpau-richard mrpau-richard deleted the hotfix/3704-cannot-connect-to-server-for-downloading-video-files-from-current-development-installs branch July 20, 2015 01:45
…ownloading-video-files-from-current-development-installs
@benjaoming benjaoming mentioned this pull request Jul 20, 2015
5 tasks
cpauya added 3 commits July 21, 2015 07:11
…ownloading-video-files-from-current-development-installs
…ownloading-video-files-from-current-development-installs
…ownloading-video-files-from-current-development-installs
@@ -51,36 +50,6 @@ def call_command_with_output(cmd, *args, **kwargs):
sys.exit = backups[2]


def call_command_subprocess(cmd, *args, **kwargs):

This comment was marked as spam.

benjaoming added a commit that referenced this pull request Jul 22, 2015
…ver-for-downloading-video-files-from-current-development-installs

Fix for PyRun: multiprocessing library does not exist, so opting for a different solution for calling external commands.
@benjaoming benjaoming merged commit c4b1f1e into learningequality:0.14.x Jul 22, 2015
@benjaoming benjaoming removed the has PR label Jul 22, 2015
@benjaoming
Copy link
Contributor

I think it's about time we merge this so we don't have stuff critical lying around unmerged just before launch!

In a more general setting: Trying to import a module or an object from a module and catching ImportError is normally a better pattern for these cases.

But this python-packages/fle_utils/django_utils/command.py is terrible and should go, I opened #4139 for this.

@cpauya
Copy link
Contributor Author

cpauya commented Jul 22, 2015

Thanks @benjaoming - looking forward for the refactor at #4139 later. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants