You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2018/07/02 18:01:21 UTC
allura git commit: [#8214] determine merge request commits as
background task
Repository: allura
Updated Branches:
refs/heads/db/8214 [created] ec110c9e2
[#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/ec110c9e
Tree: http://git-wip-us.apache.org/repos/asf/allura/tree/ec110c9e
Diff: http://git-wip-us.apache.org/repos/asf/allura/diff/ec110c9e
Branch: refs/heads/db/8214
Commit: ec110c9e28d556a226a304de2252d059b998f062
Parents: d13c882
Author: Dave Brondsema <da...@brondsema.net>
Authored: Fri Jun 29 17:32:15 2018 -0400
Committer: Dave Brondsema <da...@brondsema.net>
Committed: Mon Jul 2 14:01:17 2018 -0400
----------------------------------------------------------------------
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/ec110c9e/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/ec110c9e/Allura/allura/model/repository.py
----------------------------------------------------------------------
diff --git a/Allura/allura/model/repository.py b/Allura/allura/model/repository.py
index 4724165..b08beab 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/ec110c9e/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/ec110c9e/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/ec110c9e/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):