Code review comment for lp:~dooferlad/linaro-license-protection/push_support

Hi James!

Awesome progress! Everything looks good, there are only two definitely minor comments and one question from me.

First comes the question.
To upload a file there is no "api/" URL, we just use the catch-all urls.py rule and differentiate on GET/POST. Is that right?

So, an imaginary URL like 'https://snapshots.l.o/a_big_file.zip', with my API key and file contents as data, and a POST request, should upload the file. Is it correct?

=== added file 'license_protected_downloads/common.py'
--- license_protected_downloads/common.py 1970-01-01 00:00:00 +0000
+++ license_protected_downloads/common.py 2013-06-14 12:41:40 +0000
@@ -0,0 +1,18 @@
+import os
+
+def safe_path_join(base_path, *paths):
+ """os.path.join with with check that result is inside base_path.

s/with with/with

=== modified file 'license_protected_downloads/views.py'
+
+ # if key is in request.GET["key"] then need to mod path and give
+ # access to a per-key directory.
+ if "key" in request.GET :

Extra space before the colon?

=== modified file 'settings.py'
--- settings.py 2013-06-10 16:03:50 +0000
+++ settings.py 2013-06-14 12:41:40 +0000
 TEMPLATE_CONTEXT_PROCESSORS = (
     'django.contrib.messages.context_processors.messages',
@@ -224,3 +227,7 @@
     "init.css": "http://www.linaro.org/remote/css/init.css",
     "remote.css": "http://www.linaro.org/remote/css/remote.css",
     }
+
+MASTER_API_KEY = ""
+
+from local_settings import *

Maybe is worth adding some comments here about the MASTER_API_KEY, and also update the installation/deployment notes.

Approving since those things can be fixed during merge (and the questions are just to make sure I understood the implementation correctly).
Thanks.