allura-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kentontay...@apache.org
Subject allura git commit: [#8214] determine merge request commits as background task
Date Mon, 09 Jul 2018 16:28:26 GMT
Repository: allura
Updated Branches:
  refs/heads/master 6b5b8c527 -> 650e181ce


[#8214] determine merge request commits as background task


Project: http://git-wip-us.apache.org/repos/asf/allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/allura/commit/650e181c
Tree: http://git-wip-us.apache.org/repos/asf/allura/tree/650e181c
Diff: http://git-wip-us.apache.org/repos/asf/allura/diff/650e181c

Branch: refs/heads/master
Commit: 650e181ceb9cec3a98e600c8f864bb3935264735
Parents: 6b5b8c5
Author: Dave Brondsema <dave@brondsema.net>
Authored: Fri Jun 29 17:32:15 2018 -0400
Committer: Kenton Taylor <ktaylor@slashdotmedia.com>
Committed: Mon Jul 9 15:44:53 2018 +0000

----------------------------------------------------------------------
 Allura/allura/controllers/repository.py         | 38 +++++++++---
 Allura/allura/model/repository.py               | 20 +++---
 Allura/allura/tasks/repo_tasks.py               |  7 +++
 Allura/allura/templates/repo/merge_request.html | 39 ++++++++++--
 .../tests/functional/test_controllers.py        | 65 +++++++++++++++-----
 5 files changed, 127 insertions(+), 42 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/allura/blob/650e181c/Allura/allura/controllers/repository.py
----------------------------------------------------------------------
diff --git a/Allura/allura/controllers/repository.py b/Allura/allura/controllers/repository.py
index 2c24dbe..72de27c 100644
--- a/Allura/allura/controllers/repository.py
+++ b/Allura/allura/controllers/repository.py
@@ -401,16 +401,23 @@ class MergeRequestController(object):
             limit=limit,
             count=self.req.discussion_thread.post_count,
             subscribed=subscribed,
+            commits_task_started=False,
         )
-        try:
-            result['commits'] = self.req.commits
-        except Exception:
-            log.info(
-                "Can't get commits for merge request %s",
-                self.req.url(),
-                exc_info=True)
+        if self.req.new_commits is not None:
+            try:
+                result['commits'] = self.req.commits
+            except Exception:
+                log.info(
+                    "Can't get commits for merge request %s",
+                    self.req.url(),
+                    exc_info=True)
+                result['commits'] = []
+                result['error'] = True
+        else:
+            if self.req.commits_task_status() not in ('busy', 'ready'):
+                allura.tasks.repo_tasks.determine_mr_commits.post(self.req._id)
             result['commits'] = []
-            result['error'] = True
+            result['commits_task_started'] = True
         return result
 
     @property
@@ -515,6 +522,21 @@ class MergeRequestController(object):
         """Return result from the cache. Used by js, after task was completed."""
         return {'can_merge': self.req.can_merge()}
 
+    @expose()
+    def commits_html(self, **kw):
+        if self.req.new_commits is not None:
+            with self.req.push_downstream_context():
+                downstream_app = c.app
+            return SCMLogWidget().display(value=self.req.commits, app=downstream_app)
+
+        task_status = self.req.commits_task_status()
+        if task_status is None:
+            raise exc.HTTPNotFound
+        elif task_status == 'error':
+            raise exc.HTTPInternalServerError
+        elif task_status in ('busy', 'ready'):
+            raise exc.HTTPAccepted
+
     @expose('json:')
     @require_post()
     @validate(subscribe_form)

http://git-wip-us.apache.org/repos/asf/allura/blob/650e181c/Allura/allura/model/repository.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/repository.py b/Allura/allura/model/repository.py
index 8ec5959..493f38e 100644
--- a/Allura/allura/model/repository.py
+++ b/Allura/allura/model/repository.py
@@ -953,10 +953,10 @@ class MergeRequest(VersionedArtifact, ActivityObject):
             from allura.tasks import repo_tasks
             repo_tasks.merge.post(self._id)
 
-    def merge_task_status(self):
+    def _task_status(self, task_name):
         task = MonQTask.query.find({
             'state': {'$in': ['busy', 'complete', 'error', 'ready']},  # needed to use index
-            'task_name': 'allura.tasks.repo_tasks.merge',
+            'task_name': task_name,
             'args': [self._id],
             'time_queue': {'$gt': datetime.utcnow() - timedelta(days=1)},  # constrain on
index further
         }).sort('_id', -1).limit(1).first()
@@ -964,16 +964,14 @@ class MergeRequest(VersionedArtifact, ActivityObject):
             return task.state
         return None
 
+    def merge_task_status(self):
+        return self._task_status('allura.tasks.repo_tasks.merge')
+
     def can_merge_task_status(self):
-        task = MonQTask.query.find({
-            'state': {'$in': ['busy', 'complete', 'error', 'ready']},  # needed to use index
-            'task_name': 'allura.tasks.repo_tasks.can_merge',
-            'args': [self._id],
-            'time_queue': {'$gt': datetime.utcnow() - timedelta(days=1)},  # constrain on
index further
-        }).sort('_id', -1).limit(1).first()
-        if task:
-            return task.state
-        return None
+        return self._task_status('allura.tasks.repo_tasks.can_merge')
+
+    def commits_task_status(self):
+        return self._task_status('allura.tasks.repo_tasks.determine_mr_commits')
 
     def add_meta_post(self, changes):
         tmpl = g.jinja2_env.get_template('allura:templates/repo/merge_request_changed.html')

http://git-wip-us.apache.org/repos/asf/allura/blob/650e181c/Allura/allura/tasks/repo_tasks.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tasks/repo_tasks.py b/Allura/allura/tasks/repo_tasks.py
index f84de1a..41a1ba7 100644
--- a/Allura/allura/tasks/repo_tasks.py
+++ b/Allura/allura/tasks/repo_tasks.py
@@ -169,3 +169,10 @@ def can_merge(merge_request_id):
     mr = M.MergeRequest.query.get(_id=merge_request_id)
     result = mr.app.repo.can_merge(mr)
     mr.set_can_merge_cache(result)
+
+
+@task
+def determine_mr_commits(merge_request_id):
+    from allura import model as M
+    mr = M.MergeRequest.query.get(_id=merge_request_id)
+    mr.commits  # build & cache the commits

http://git-wip-us.apache.org/repos/asf/allura/blob/650e181c/Allura/allura/templates/repo/merge_request.html
----------------------------------------------------------------------
diff --git a/Allura/allura/templates/repo/merge_request.html b/Allura/allura/templates/repo/merge_request.html
index c408ba4..2b8e70d 100644
--- a/Allura/allura/templates/repo/merge_request.html
+++ b/Allura/allura/templates/repo/merge_request.html
@@ -119,9 +119,16 @@ Merge Request #{{req.request_number}}: {{req.summary}} ({{req.status}})
 
     </div>
 
-    {{ c.log_widget.display(value=commits, app=downstream_app) }}
-
-    <div class="grid-19"><a href="#discussion_holder">Discuss</a></div>
+    {% if commits_task_started %}
+        <div class='grid-19 commits-loading'>
+            <p>
+            <img src="{{g.forge_static('images/spinner.gif')}}" class="spinner"/>
+            Determining commits...
+            </p>
+        </div>
+    {% else %}
+        {{ c.log_widget.display(value=commits, app=downstream_app) }}
+    {% endif %}
 
     {% if h.has_access(c.app, 'write')() %}
       <div class="grid-19">
@@ -171,6 +178,7 @@ Merge Request #{{req.request_number}}: {{req.summary}} ({{req.status}})
   .merge-conflicts { color: red; }
   .can-merge-in-progress { color: grey; }
   .merge-instructions { width:80%; height:60px; }
+  .merge-toolbar { padding-bottom: 1em; }
 
   #merge_task_status .{{ merge_status }} { display: inline-block; }
   #can_merge_task_status .{{ can_merge_status }} { display: inline-block; }
@@ -186,9 +194,10 @@ Merge Request #{{req.request_number}}: {{req.summary}} ({{req.status}})
 {{ super() }}
 <script type="text/javascript">
 $(function() {
+    var delay = 500;
+    var delay_threshold = 60000;
+
     function make_status_watcher(spinner_selector, status_url, on_complete, on_progress,
on_error) {
-      var delay = 500;
-      var delay_threshold = 60000;
 
       var check_status = function() {
         $.get(status_url, function(data) {
@@ -203,7 +212,7 @@ $(function() {
               on_error();
             }
             if (delay < delay_threshold) {
-              delay = delay * 2;
+              delay = delay * 1.5;
             }
             window.setTimeout(check_status, delay);
           }
@@ -269,6 +278,24 @@ $(function() {
       var start_watcher = make_status_watcher(spinner, url, on_complete, on_progress, on_error);
       start_watcher();
     {% endif %}
+
+    {% if commits_task_started %}
+      var check_commits = function() {
+        $.get('{{ request.path.rstrip('/') }}/commits_html', function(data, textStatus, jqXHR)
{
+            if (jqXHR.status === 200) {
+                $('.commits-loading').replaceWith(data);
+            } else if (jqXHR.status === 500) {
+                $('.commits-loading').replaceWith('<p>An error occurred while determining
the commits in this merge request.</p>')
+            } else {
+                if (delay < delay_threshold) {
+                    delay = delay * 1.5;
+                }
+                window.setTimeout(check_commits, delay);
+            }
+        });
+      };
+      check_commits();
+    {% endif %}
 });
 </script>
 {% endblock %}

http://git-wip-us.apache.org/repos/asf/allura/blob/650e181c/ForgeGit/forgegit/tests/functional/test_controllers.py
----------------------------------------------------------------------
diff --git a/ForgeGit/forgegit/tests/functional/test_controllers.py b/ForgeGit/forgegit/tests/functional/test_controllers.py
index b4b6f9b..fb4377d 100644
--- a/ForgeGit/forgegit/tests/functional/test_controllers.py
+++ b/ForgeGit/forgegit/tests/functional/test_controllers.py
@@ -619,21 +619,12 @@ class TestFork(_TestCase):
 
     def test_merge_request_detail_view(self):
         r, mr_num = self._request_merge()
-        assert 'wants to merge' in r, r.showbrowser()
-        assert 'Improve documentation' in r, r.showbrowser()
-        revs = r.html.findAll('tr', attrs={'class': 'rev'})
-        assert_equal(len(revs), 1)
-        rev_links = revs[0].findAll('a', attrs={'class': 'rev'})
-        browse_links = revs[0].findAll('a', attrs={'class': 'browse'})
-        c_id = self.forked_repo.get_heads()[0]['object_id']
-        assert_equal(rev_links[0].get('href'), '/p/test2/code/ci/%s/' % c_id)
-        assert_equal(rev_links[0].getText(), '[%s]' % c_id[:6])
-        assert_equal(browse_links[0].get('href'),
-                     '/p/test2/code/ci/%s/tree' % c_id)
-        assert_equal(browse_links[0].getText(), 'Tree')
+        assert_in('wants to merge', r)
+
         merge_instructions = r.html.findAll('textarea')[0].getText()
         assert_in('git checkout master', merge_instructions)
         assert_in('git fetch /srv/git/p/test2/code master', merge_instructions)
+        c_id = self.forked_repo.get_heads()[0]['object_id']
         assert_in('git merge {}'.format(c_id), merge_instructions)
         assert_regexp_matches(str(r), r'[0-9]+ seconds? ago')
 
@@ -641,6 +632,33 @@ class TestFork(_TestCase):
         assert merge_form
         assert_in('Merge request has no conflicts. You can merge automatically.', merge_form.getText())
 
+        assert_not_in('Improve documentation', r)  # no details yet
+
+        # a task is busy/ready to compute
+        r = self.app.get('/p/test/src-git/merge-requests/1/commits_html', status=202)  #
202 used for "busy"
+        # run task to compute the commits list
+        task = M.MonQTask.query.get(task_name='allura.tasks.repo_tasks.determine_mr_commits',
state='ready')
+        task()
+        ThreadLocalORMSession.close_all()  # close ming connections so that new data gets
loaded later
+
+        def assert_commit_details(r):
+            assert_in('Improve documentation', r.body)
+            revs = r.html.findAll('tr', attrs={'class': 'rev'})
+            assert_equal(len(revs), 1)
+            rev_links = revs[0].findAll('a', attrs={'class': 'rev'})
+            browse_links = revs[0].findAll('a', attrs={'class': 'browse'})
+            assert_equal(rev_links[0].get('href'), '/p/test2/code/ci/%s/' % c_id)
+            assert_equal(rev_links[0].getText(), '[%s]' % c_id[:6])
+            assert_equal(browse_links[0].get('href'),
+                         '/p/test2/code/ci/%s/tree' % c_id)
+            assert_equal(browse_links[0].getText(), 'Tree')
+
+        r = self.app.get('/p/test/src-git/merge-requests/1/commits_html', status=200)
+        assert_commit_details(r)
+
+        r = self.app.get('/p/test/src-git/merge-requests/1/', status=200)
+        assert_commit_details(r)
+
     def test_merge_request_detail_noslash(self):
         self._request_merge()
         r = self.app.get('/p/test/src-git/merge-requests/1', status=301)
@@ -691,20 +709,32 @@ class TestFork(_TestCase):
         assert_equal(_select_val(r, 'target_branch'), 'zz')
 
     def test_merge_request_with_branch(self):
+        def get_mr_page(r):
+            r = r.follow()  # get merge request page; creates bg task for determining commits
+            task = M.MonQTask.query.get(task_name='allura.tasks.repo_tasks.determine_mr_commits',
state='ready')
+            task()
+            ThreadLocalORMSession.close_all()  # close ming connections so that new data
gets loaded later
+            r = self.app.get(r.request.url)  # refresh, data should be there now
+            return r
+
         r = self.app.post('/p/test2/code/do_request_merge',
                           params={
                               'source_branch': 'zz',
                               'target_branch': 'zz',
                               'summary': 'summary',
-                              'description': 'description'}).follow()
+                              'description': 'description'})
+        r = get_mr_page(r)
         assert '[5c4724]' not in r
+
+        # again with different branch
         r = self.app.post('/p/test2/code/do_request_merge',
                           params={
                               'source_branch': 'zz',
                               'target_branch': 'master',
                               'summary': 'summary',
-                              'description': 'description'}).follow()
-        assert '[5c4724]' in r
+                              'description': 'description'})
+        r = get_mr_page(r)
+        assert '[5c4724]' in r, r
 
     def test_merge_request_edit(self):
         r = self.app.post('/p/test2/code/do_request_merge',
@@ -775,8 +805,9 @@ class TestFork(_TestCase):
         r, mr_num = self._request_merge()
         mr_commits.side_effect = Exception
         r = self.app.get('/p/test/src-git/merge-requests/%s/' % mr_num)
-        err = r.html.find('div', attrs={'class': 'grid-19 error'})
-        assert_in("Can't find commits to merge", err.getText())
+        # errors don't show up on the page directly any more, so just assert that the bg
task is there
+        assert_in('commits-loading', r)
+        self.app.get('/p/test/src-git/merge-requests/%s/commits_html' % mr_num, status=202)
 # 202 used for "busy"
 
 
 class TestDiff(TestController):


Mime
View raw message