From bff8cd4e4ee96e09c74d9faf2c20b6c9ee790831 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 10 Sep 2012 10:10:25 -0400 Subject: [PATCH 1/5] don't break courseware if S3 is unreachable --- common/lib/xmodule/xmodule/course_module.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index f00a22782c..4a009dcae4 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -99,7 +99,14 @@ class CourseDescriptor(SequenceDescriptor): def definition_from_xml(cls, xml_object, system): textbooks = [] for textbook in xml_object.findall("textbook"): - textbooks.append(cls.Textbook.from_xml_object(textbook)) + try: + txt = cls.Textbook.from_xml_object(textbook) + except: + # If we can't get to S3 (e.g. on a train with no internet), don't break + # the rest of the courseware. + log.exception("Couldn't load textbook") + continue + textbooks.append() xml_object.remove(textbook) #Load the wiki tag if it exists From 814cd560344f38a9394c604b761c02e68ec50900 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 10 Sep 2012 10:38:17 -0400 Subject: [PATCH 2/5] return proper error codes for 404 and 500 errors --- lms/djangoapps/static_template_view/views.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/static_template_view/views.py b/lms/djangoapps/static_template_view/views.py index 90087e06d6..8ab6216eda 100644 --- a/lms/djangoapps/static_template_view/views.py +++ b/lms/djangoapps/static_template_view/views.py @@ -6,6 +6,7 @@ from mitxmako.shortcuts import render_to_response, render_to_string from django.shortcuts import redirect from django.conf import settings +from django.http import HttpResponseNotFound, HttpResponseServerError from django_future.csrf import ensure_csrf_cookie from util.cache import cache_if_anonymous @@ -40,9 +41,9 @@ def render(request, template): def render_404(request): - return render_to_response('static_templates/404.html', {}) + return HttpResponseNotFound(render_to_string('static_templates/404.html', {})) def render_500(request): - return render_to_response('static_templates/server-error.html', {}) + return HttpResponseServerError(render_to_string('static_templates/server-error.html', {})) From 779f86569128e20d857a655a911dda61af362cf2 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 10 Sep 2012 10:49:12 -0400 Subject: [PATCH 3/5] remove check-for-404 hack from tests now that we get real 404s --- lms/djangoapps/courseware/tests/tests.py | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/lms/djangoapps/courseware/tests/tests.py b/lms/djangoapps/courseware/tests/tests.py index 62456d65d5..f3b086748d 100644 --- a/lms/djangoapps/courseware/tests/tests.py +++ b/lms/djangoapps/courseware/tests/tests.py @@ -222,16 +222,9 @@ class PageLoader(ActivateLoginTestCase): handling. """ resp = self.client.get(url) - # HACK: workaround the bug that returns 200 instead of 404. - # TODO (vshnayder): once we're returning 404s, get rid of this if. - if code != 404: - self.assertEqual(resp.status_code, code) - # And 'page not found' shouldn't be in the returned page - self.assertTrue(resp.content.lower().find('page not found') == -1) - else: - # look for "page not found" instead of the status code - #print resp.content - self.assertTrue(resp.content.lower().find('page not found') != -1) + self.assertEqual(resp.status_code, code, + "got code {0} for url '{1}'. Expected code {2}" + .format(resp.status_code, url, code)) def check_pages_load(self, course_name, data_dir, modstore): From 317e2c345e028cb12c1d95b4962615cba30ecb44 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Mon, 10 Sep 2012 11:31:37 -0400 Subject: [PATCH 4/5] Add autodeploy properties task to rakefile --- rakefile | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/rakefile b/rakefile index 053abf56a8..9e0bbcbfa4 100644 --- a/rakefile +++ b/rakefile @@ -227,3 +227,13 @@ namespace :cms do end end end + +desc "Build a properties file used to trigger autodeploy builds" +task :autodeploy_properties do + File.open("autodeploy.properties", "w") do |file| + file.puts("UPSTREAM_NOOP=false") + file.puts("UPSTREAM_BRANCH=#{BRANCH}") + file.puts("UPSTREAM_JOB=#{PACKAGE_NAME}") + file.puts("UPSTREAM_REVISION=#{COMMIT}") + end +end From 9266bcca6d4e6a0e5af10f227a62d122a7bfa6e0 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Mon, 10 Sep 2012 17:42:01 -0400 Subject: [PATCH 5/5] fix stupid bug in textbook handling code --- common/lib/xmodule/xmodule/course_module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 4a009dcae4..7aa904205d 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -106,7 +106,7 @@ class CourseDescriptor(SequenceDescriptor): # the rest of the courseware. log.exception("Couldn't load textbook") continue - textbooks.append() + textbooks.append(txt) xml_object.remove(textbook) #Load the wiki tag if it exists