Merge lp:~dooferlad/linaro-android-frontend/ui_download_links_update into lp:linaro-android-frontend

[James Tunnicliffe]

UI will present the user a list of downloads. It will test to see if it can find a build on snapshots.linaro.org. If it finds it it will use that server and point the links there. If it fails to find a build on snapshots, it will look for a locally stored on android-build.linaro.org. If it finds a build there, it will point the download links to it. If it fails to find builds anywhere, it will point links to where it thinks the build should be on snapshots.

In preparation for artefact publishing going away this update also supports using a file called manifest.txt, which is just a list of files (one relative location per line). If it finds manifest.txt in the root of the download location, it will use that. It will fall back to the artefact list and if that fails, display a message saying it couldn't provide links to individual files.

I have tested this using a proxy that would replace bits of the live android-build.linaro.org site (https://code.launchpad.net/~dooferlad/junk/dev_proxy) so I could browse around several build pages to see if this worked. Unfortunately it looks like my ISP is suffering from a routing problem and I just lost connectivity to android-build.linaro.org and I didn't have a chance to test looking at a build that has its downloads on android-build.linaro.org. It should work, but at this point it would be nice to have some feedback on the change.

[lp:~dooferlad/linaro-android-frontend/ui_download_links_update]

251. By James Tunnicliffe on 2012-01-18

Fixed error - wasn't testing return code when examinining where a build was hosted.
Fixed test_remote_dir_exists description.
Allow test_remote_dir_exists to test android-build.linaro.org.

[James Tunnicliffe (dooferlad)]

A couple of fixes added and now tested in the following conditions:

1. Build exists on android-build.linaro.org only
   - File links point to android-build.linaro.org
2. Build exists on snapshots.linaro.org
   - File links point to snapshots.linaro.org
3. Build exists on shapshots.linaro.org and manifest.txt exists
   - File links point to snapshots.linaro.org and download list comes from
     manifest.txt

So it should be good to go.

[Paul Sokolovsky (pfalcon)]

Well, comments are:

1. Are you sure that making 3 synchronous AJAX requests doesn't hurt page download/interactive performance?

2. create_manifest.py script apparently doesn't belong here, but rather should be part of linaro-android-build-tools and be called from build-android script at the appropriate place.

3. Did you give extra thought what manifest.txt may contain? Just filenames? Maybe file size? What about MD5SUMS file we already create for subset of files (actually, one of the reason why is it created for subset and not all was because that would require faithful duplication of artifact filename patterns - both in Jenkins and in this script, same problem would exist for create_manifest.py too). (I don't have any specific suggestion regarding what manifest.txt should include, just wanted to make sure this is though-out).

4. Ah, last one, naming. Maybe "MANIFEST" is better, it's kind of common practice (that depends on contents format of course).

review: Needs Information

[James Tunnicliffe (dooferlad)]

Hi Paul, thanks for the comments.

> 1. Are you sure that making 3 synchronous AJAX requests doesn't hurt page download/interactive performance?

It hasn't caused any obvious performance problems for me, but if one
or more of the requests took a long time it could be a problem. Will
investigate how easy it is to move them to asynchronous requests.

> 2. create_manifest.py script apparently doesn't belong here, but rather should be part of linaro-android-build-tools and be called from build-android script at the appropriate place.

Ah, thanks. Will move it.

> 3. Did you give extra thought what manifest.txt may contain? Just filenames? Maybe file size? What about MD5SUMS file we already create for subset of files (actually, one of the reason why is it created for subset and not all was because  that would require faithful duplication of artifact filename patterns - both in Jenkins and in this script, same problem would exist for create_manifest.py too). (I don't have any specific suggestion regarding what manifest.txt should include, just wanted to make sure this is though-out).

The only thing the web UI will use is file names and that is the only
reason to add the file. If we extend its use later we could update the
UI to cope.

> 4. Ah, last one, naming. Maybe "MANIFEST" is better, it's kind of common practice (that depends on contents format of course).

Not a problem - changing a file name is easy enough.

--
James Tunnicliffe

[Paul Sokolovsky (pfalcon)]

On Mon, 23 Jan 2012 16:42:47 -0000
James Tunnicliffe <email address hidden> wrote:

> Hi Paul, thanks for the comments.
>
> > 1. Are you sure that making 3 synchronous AJAX requests doesn't
> > hurt page download/interactive performance?
>
> It hasn't caused any obvious performance problems for me, but if one
> or more of the requests took a long time it could be a problem. Will
> investigate how easy it is to move them to asynchronous requests.

Well, my idea of that would be done was to have chain of async result
handlers - i.e. result handler of 1st request would check if it got
what's needed and if so, display it, otherwise queue up another request
whose handler would do similarly, etc.

I don't pledge for sync processing to be changed immediately, just want
to make sure it is known as point of potential problem and we're ready
to resolve it.

>
> > 2. create_manifest.py script apparently doesn't belong here, but
> > rather should be part of linaro-android-build-tools and be called
> > from build-android script at the appropriate place.
>
> Ah, thanks. Will move it.
>
> > 3. Did you give extra thought what manifest.txt may contain? Just
> > filenames? Maybe file size? What about MD5SUMS file we already
> > create for subset of files (actually, one of the reason why is it
> > created for subset and not all was because  that would require
> > faithful duplication of artifact filename patterns - both in
> > Jenkins and in this script, same problem would exist for
> > create_manifest.py too). (I don't have any specific suggestion
> > regarding what manifest.txt should include, just wanted to make
> > sure this is though-out).
>
> The only thing the web UI will use is file names and that is the only
> reason to add the file. If we extend its use later we could update the
> UI to cope.

Makes sense.

>
> > 4. Ah, last one, naming. Maybe "MANIFEST" is better, it's kind of
> > common practice (that depends on contents format of course).
>
> Not a problem - changing a file name is easy enough.

Ok, so I assume there will be more commits for this merge request
before approving it.

Thanks,
Paul

Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

[lp:~dooferlad/linaro-android-frontend/ui_download_links_update]

252. By James Tunnicliffe on 2012-01-24

create_manifest.py moved to linaro-android-build-tools project.
Add android-build.linaro.org as a proxy location
Updated static/buildDetails.js so lava-job-info and MANIFEST files are both sourced from the location returned by getDownloadURL.

TODO: remove as many synchronous Y.io calls as possible. It should be possible to save the result of getDownloadURL and reuse it.

253. By James Tunnicliffe on 2012-01-24

getDownloadURL - cache result of last call to reduce remote requests. The cached value is deleted on URL change since the JS doesn't reload, so the same cached value would be returned for different builds of the same project without this step.

[James Tunnicliffe (dooferlad)]

I have just pushed a new version. This doesn't perform asynchronous
requests, but does change the name of manifest.txt to MANIFEST and fix
a couple of bugs.

James

[Paul Sokolovsky (pfalcon)]

review: Approve

=== modified file 'android_build/frontend/api.py'
--- android_build/frontend/api.py	2012-01-13 16:17:34 +0000
+++ android_build/frontend/api.py	2012-01-24 14:42:25 +0000
@@ -255,7 +255,19 @@
         return resp
     return HttpResponse('foo')
 
-def get_lava_job_info(request):
+def _match_host(allowed_hosts, url):
+    for host in allowed_hosts:
+        if re.search("^" + host, url):
+            return True
+    return False
+
+def _match_file(allowed_files, url):
+    for file in allowed_files:
+        if re.search(file + "$", url):
+            return True
+    return False
+
+def proxy_remote_file(request):
     """Give Web UI a proxy to fetch lava-job-info for it to parse.
 
     This is required because the android files are now protected by the need
@@ -269,8 +281,13 @@
     for thing in request.GET.getlist("url"):
         url = thing
 
-    if not (re.search("/lava-job-info$", url) and
-            re.search("^https?://snapshots.linaro.org/", url)):
+    allowed_hosts = ["https?://snapshots.linaro.org/",
+                     "https?://android-build.linaro.org"]
+
+    allowed_files = ['lava-job-info', 'manifest.txt']
+
+    if not (_match_host(allowed_hosts, url) and
+            _match_file(allowed_files, url)):
         raise Http404
 
     try:
@@ -289,3 +306,38 @@
         return HttpResponse(message)
     else:
         return HttpResponse(response.read())
+
+def test_remote_dir_exists(request):
+    """Give Web UI the ability to test where a build is stored.
+
+    Checks if a directory exists. If it does, return OK (200), else, raise
+    a 404.
+    """
+
+    url = ""
+    for thing in request.GET.getlist("url"):
+        url = thing
+
+    allowed_hosts = ["https?://snapshots.linaro.org/",
+                     "https?://android-build.linaro.org"]
+
+    if not (_match_host(allowed_hosts, url) and
+            re.search('/$', url)):
+        raise Http404
+
+    try:
+        urllib2.urlopen(url)
+    except urllib2.HTTPError, e:
+        message = "Server error: %d" % e.code
+        resp = HttpResponse(message)
+        resp.status_code = e.code
+        return resp
+    except urllib2.URLError, e:
+        message = "We failed to reach a server."
+
+        if isinstance(e.reason, str):
+            message += " Reason %s" % e.reason
+
+        return HttpResponse(message)
+    else:
+        return HttpResponse("Found")

=== modified file 'android_build/urls.py'
--- android_build/urls.py	2012-01-09 15:56:08 +0000
+++ android_build/urls.py	2012-01-24 14:42:25 +0000
@@ -27,7 +27,8 @@
     (r'^api/is-daily$', 'android_build.frontend.api.is_daily'),
     (r'^api/new$', 'android_build.frontend.api.new'),
     (r'^api/set-daily$', 'android_build.frontend.api.set_daily'),
-    (r'^api/get-lava-job-info$', 'android_build.frontend.api.get_lava_job_info'),
+    (r'^api/proxy-remote-file$', 'android_build.frontend.api.proxy_remote_file'),
+    (r'^api/test-dir-exists$', 'android_build.frontend.api.test_remote_dir_exists'),
 
     (r'^openid/', include('django_openid_auth.urls')),
     (r'^logout$', 'django.contrib.auth.views.logout'),

=== modified file 'static/buildDetails.js'
--- static/buildDetails.js	2012-01-13 17:01:54 +0000
+++ static/buildDetails.js	2012-01-24 14:42:25 +0000
@@ -8,6 +8,11 @@
       'history:change',
       function (e) {
         if (e.src === Y.HistoryHash.SRC_HASH) {
+          // URL changed
+          if (typeof getDownloadURL.downloadUrl != "undefined")
+          {
+            delete getDownloadURL.downloadUrl;
+          }
           displayJobByNumber(e.changed.build.newVal);
         }
       }
@@ -120,6 +125,56 @@
         );
     }
 
+    function getProxyURL ()
+    {
+      return '/api/proxy-remote-file?url=';
+    }
+
+    function getDownloadURL(results)
+    {
+      /* Work out where to download from. The assumption is that all
+       downloads will end up on snapshots.linaro.org soon, but we want to
+       detect if they haven't moved. If we can't find a build on
+       snapshots and we can find it locally, point the download location
+       to the local build. If we can't find it locally either, keep pointing
+       at snapshots.
+       */
+
+      if (typeof getDownloadURL.downloadUrl != "undefined")
+      {
+        return getDownloadURL.downloadUrl; // Return cached result
+      }
+
+      var downloadUrl = 'http://snapshots.linaro.org/android/' + rawBuildName + '/' + results.number.toString() + '/';
+
+      var test_snapshots = Y.io(
+        '/api/test-dir-exists?url=' + downloadUrl,
+        {
+          sync: true
+        });
+
+      if (test_snapshots.status != 200) // snapshots.linaro.org doesn't host this build
+      {
+        // Test to see if the local server (probably android-build.linaro.org) does.
+        var testUrl = 'http://android-build.linaro.org/builds/' + rawBuildName + '/' + results.number.toString() + '/';
+
+        var test_local = Y.io(
+          '/api/test-dir-exists?url=' + testUrl,
+          {
+            sync: true
+          });
+
+        if (test_local.status == 200)
+        {
+          // Found a build host - update download link.
+          downloadUrl = testUrl;
+        }
+      }
+
+      getDownloadURL.downloadUrl = downloadUrl; // Save result to speed up future calls
+      return downloadUrl;
+    }
+
     var element_mapping = {
       status: function (results) {
         return results.result || "RUNNING";
@@ -157,11 +212,10 @@
       },
       testresults: function (results) {
 
-          var new_style_url = '/api/get-lava-job-info?url=http://snapshots.linaro.org/android/'
-              + rawBuildName + '/' + results.number.toString() + '/lava-job-info';
+          var downloadUrl = getDownloadURL(results);
 
           Y.io(
-              new_style_url,
+              getProxyURL() + downloadUrl + '/lava-job-info',
               {
                   on: { success: receivedLavaJobInfo,
                         failure: function(id, resp) {
@@ -172,14 +226,54 @@
 
           return 'Loading...';
       },
+
       output: function (results) {
-        var downloadAreaUrl = 'http://snapshots.linaro.org/android/' + rawBuildName + '/' + results.number.toString() + '/';
-        Y.one('#all-output a').setAttribute(
-          'href', downloadAreaUrl);
+        /* Generate a download list */
+
+        /* In case getDownloadURL fails for some reason, we always have a link
+           to where new builds should be.
+         */
+        var downloadUrl = 'http://snapshots.linaro.org/android/' + rawBuildName + '/' + results.number.toString() + '/';
+
+        Y.one('#all-output a').setAttribute('href', downloadUrl);
         Y.one('#all-output a').removeClass('hidden');
         Y.one('#all-output span').addClass('hidden');
-        if (results.artifacts.length) {
-          var downloadUrl = 'http://' + window.location.hostname + '/builds/' + rawBuildName + '/' + results.number.toString() + '/';
+
+        downloadUrl = getDownloadURL(results);
+
+        /* Decision made about downloadUrl. Now use it. We test to see if we
+           can find manifest.txt, which is a list of all files available for
+           download. If we can't find it we try using results.artifacts.
+           If that fails, we display a message saying that we can't provide
+           a downloads list.
+         */
+        var io_result = Y.io(
+          getProxyURL() + downloadUrl + '/MANIFEST',
+          {
+            sync: true
+          });
+
+        if (io_result.status == 200)
+        {
+          var manifest = io_result.responseText.split("\n");
+
+          var listNode = Y.Node.create('<ul/>');
+
+          for (var i = 0; i < manifest.length; i++)
+          {
+            if (!manifest[i].match(/\S/))
+            {
+              continue;
+            }
+            var itemNode = Y.Node.create('<li><a/></li>');
+            itemNode.one('a').setContent(manifest[i]);
+            itemNode.one('a').setAttribute('href', downloadUrl + manifest[i]);
+            listNode.appendChild(itemNode);
+          }
+          return listNode;
+        }
+        else if (results.artifacts.length)
+        {
           var listNode = Y.Node.create('<ul/>');
           for (var i = 0; i < results.artifacts.length; i++) {
             var artifact = results.artifacts[i];
@@ -190,10 +284,10 @@
             listNode.appendChild(itemNode);
           }
           return listNode;
-        } else {
-//          Y.one('#all-output a').addClass('hidden');
-//          Y.one('#all-output span').removeClass('hidden');
-          return Y.Node.create("<p>Build artifacts not known</p>");
+        }
+        else
+        {
+          return Y.Node.create('<p>Unable to find downloads list.</p>')
         }
       }
     };