Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add script and tests for moving a batched request to a stable request #1678

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions bodhi/client/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def _warn_if_url_and_staging_set(ctx, param, value):
click.option('--bugs', help='Comma-separated list of bug numbers', default=''),
click.option('--close-bugs', default=True, is_flag=True, help='Automatically close bugs'),
click.option('--request', help='Requested repository',
type=click.Choice(['testing', 'stable', 'unpush'])),
type=click.Choice(['testing', 'stable', 'unpush', 'batched'])),
click.option('--autokarma', is_flag=True, help='Enable karma automatism'),
click.option('--stable-karma', type=click.INT, help='Stable karma threshold'),
click.option('--unstable-karma', type=click.INT, help='Unstable karma threshold'),
Expand Down Expand Up @@ -274,7 +274,7 @@ def edit(user, password, url, **kwargs):
@click.option('--releases', help='Updates for specific releases')
@click.option('--locked', help='Updates that are in a locked state')
@click.option('--request', help='Updates with a specific request',
type=click.Choice(['testing', 'stable', 'unpush']))
type=click.Choice(['testing', 'stable', 'unpush', 'batched']))
@click.option('--submitted-since',
help='Updates that have been submitted since a certain time')
@click.option('--status', help='Filter by update status',
Expand Down Expand Up @@ -324,7 +324,7 @@ def request(update, state, user, password, url, **kwargs):
UPDATE: The title of the update (e.g. FEDORA-2017-f8e0ef2850)

STATE: The state you wish to change the update\'s request to. Valid options are
testing, stable, obsolete, unpush, and revoke.
testing, stable, obsolete, unpush, batched, and revoke.
"""

# Developer Docs
Expand Down
3 changes: 3 additions & 0 deletions bodhi/server/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,9 @@ class BodhiConfig(dict):
'value': ('%s has been pushed to the %s repository. If problems still persist, please '
'make note of it in this bug report.'),
'validator': unicode},
'stable_from_batched_msg': {
'value': ('This update has been dequeued from batched and is now entering stable.'),
'validator': unicode},
'stacks_enabled': {
'value': False,
'validator': _validate_bool},
Expand Down
21 changes: 13 additions & 8 deletions bodhi/server/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1731,8 +1731,8 @@ def set_request(self, db, action, username):

# If status is testing going to stable request and action is revoke,
# keep the status at testing
elif self.status is UpdateStatus.testing and self.request is UpdateRequest.stable \
and action is UpdateRequest.revoke:
elif self.request in (UpdateRequest.stable, UpdateRequest.batched) and \
self.status is UpdateStatus.testing and action is UpdateRequest.revoke:
self.status = UpdateStatus.testing
self.revoke()
flash_log("%s has been revoked." % self.title)
Expand All @@ -1748,7 +1748,7 @@ def set_request(self, db, action, username):
return

# Disable pushing critical path updates for pending releases directly to stable
if action is UpdateRequest.stable and self.critpath:
if action in (UpdateRequest.stable, UpdateRequest.batched) and self.critpath:
if config.get('critpath.num_admin_approvals') is not None:
if not self.critpath_approved:
stern_note = (
Expand All @@ -1774,7 +1774,7 @@ def set_request(self, db, action, username):

# Ensure this update meets the minimum testing requirements
flash_notes = ''
if action is UpdateRequest.stable and not self.critpath:
if action in (UpdateRequest.stable, UpdateRequest.batched) and not self.critpath:
# Check if we've met the karma requirements
if (self.stable_karma not in (None, 0) and self.karma >=
self.stable_karma) or self.critpath_approved:
Expand Down Expand Up @@ -2373,9 +2373,14 @@ def check_karma_thresholds(self, db, agent):
self.comment(db, text, author=u'bodhi')
elif self.stable_karma and self.karma >= self.stable_karma:
if self.autokarma:
log.info("Automatically marking %s as stable" % self.title)
self.set_request(db, UpdateRequest.stable, agent)
self.request = UpdateRequest.stable
if self.severity is UpdateSeverity.urgent or self.type is UpdateType.newpackage:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, I think this addresses the feedback from the devel list.

log.info("Automatically marking %s as stable" % self.title)
self.set_request(db, UpdateRequest.stable, agent)
else:
log.info("Automatically adding %s to batch of updates that will be pushed to"
" stable at a later date" % self.title)
self.set_request(db, UpdateRequest.batched, agent)

self.date_pushed = None
notifications.publish(
topic='update.karma.threshold.reach',
Expand Down Expand Up @@ -2571,7 +2576,7 @@ def requested_tag(self):
# release to the Release.dist-tag
if self.release.state is ReleaseState.pending:
tag = self.release.dist_tag
elif self.request is UpdateRequest.testing:
elif self.request in (UpdateRequest.testing, UpdateRequest.batched):
Copy link

@TC01 TC01 Aug 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I made this comment on the fedora devel list and was told to post it here).

It would be nice if new package updates were also sent directly to stable with something like or self.type is newpackage-- there isn't a compelling reason to batch new package updates (since they won't automatically get installed on an end-user system) and this way they don't get artificially made any slower than they already are, ensuring brand-new packages still get to users in a timely manner.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this suggestion makes sense. @crungehottman, if you can add one more if statement and one more test for new packages, that would be great!

tag = self.release.testing_tag
elif self.request is UpdateRequest.obsolete:
tag = self.release.candidate_tag
Expand Down
46 changes: 46 additions & 0 deletions bodhi/server/scripts/dequeue_stable.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# -*- coding: utf-8 -*-
# Copyright © 2017 Caleigh Runge-Hottman
#
# This file is part of Bodhi.
#
# This program is free software; you can redistribute it and/or
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a Copyright claim here. Only, I'm not quite sure who to put as the Copyright holder. I started a thread on outreachy-list to ask. If you are on that list, perhaps you can use their feedback to know what to do here. Anyways, once we know who or what organization to give the Copyright to, let's do something like this here:

# -*- coding: utf-8 -*-
# Copyright © 2017 <copyright holder here>
#
# This file is part of Bodhi.
#

and then the rest of the header like you have it here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to Tony Sebro, you get to hold the copyright on this code (neat, huh? In most places, your employer would hold the copyright on code that you are paid to write so that's pretty cool I think). So you should do this in this file and the test file you wrote:

# -*- coding: utf-8 -*-
# Copyright © 2017 Caleigh Runge-Hottman
#
# This file is part of Bodhi.
#
<the rest of the file as you have it, including the GPL stuff>

# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
"""This script is responsible for moving all updates with a batched request to a stable request."""

import sys

import click

from bodhi.server import buildsys, config, models, Session, initialize_db


@click.command()
@click.version_option(message='%(version)s')
def dequeue_stable():
"""Convert all batched requests to stable requests."""
initialize_db(config.config)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to initialize the buildsys here, otherwise the script gets an error:

[vagrant@bodhi-dev bodhi]$ bodhi-dequeue-stable 
Buildsys needs to be setup

Here's a patch that sets up the buildsys and also sets up the console script entry point:

diff --git a/bodhi/server/scripts/dequeue_stable.py b/bodhi/server/scripts/dequeue_stable.py
index 70d1ecd..4ca7d00 100644
--- a/bodhi/server/scripts/dequeue_stable.py
+++ b/bodhi/server/scripts/dequeue_stable.py
@@ -22,7 +22,7 @@ import sys
 
 import click
 
-from bodhi.server import config, models, Session, initialize_db
+from bodhi.server import buildsys, config, models, Session, initialize_db
 
 
 @click.command()
@@ -30,6 +30,7 @@ from bodhi.server import config, models, Session, initialize_db
 def dequeue_stable():
     """Convert all batched requests to stable requests."""
     initialize_db(config.config)
+    buildsys.setup_buildsystem(config.config)
     db = Session()
 
     try:
diff --git a/setup.py b/setup.py
index ba739ed..be5180a 100644
--- a/setup.py
+++ b/setup.py
@@ -154,6 +154,7 @@ setup(
     initialize_bodhi_db = bodhi.server.scripts.initializedb:main
     bodhi-babysit-ci = bodhi.server.scripts.babysit_ci:babysit
     bodhi-clean-old-mashes = bodhi.server.scripts.clean_old_mashes:clean_up
+    bodhi-dequeue-stable = bodhi.server.scripts.dequeue_stable:dequeue_stable
     bodhi-push = bodhi.server.push:push
     bodhi-expire-overrides = bodhi.server.scripts.expire_overrides:main
     bodhi-untag-branched = bodhi.server.scripts.untag_branched:main

With that patch I was able to see an update get to batched by me giving it karma, and then I was able to see it get to stable with the script. Very cool!

buildsys.setup_buildsystem(config.config)
db = Session()

try:
batched = db.query(models.Update).filter_by(request=models.UpdateRequest.batched).all()
for update in batched:
update.set_request(db, models.UpdateRequest.stable, u'bodhi')
db.commit()

except Exception as e:
print(str(e))
db.rollback()
Session.remove()
sys.exit(1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fine way to do it, but I'll mention that we also have a TransactionalSessionMaker that you can use to handle the db commit/rollback for you. Here's an example of using it:

initialize_db(config)
db_factory = transactional_session_maker()
with db_factory() as session:
    # Your code goes here, and the db_factory will automatically commit for you if no Exception is raised. It will also
    # automatically rollback if one is. Oh, and the ```session``` it gives you is the same thing as the ```db``` variable
    # you have defined, so that's handy too.

2 changes: 1 addition & 1 deletion bodhi/server/services/updates.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def set_request(request):
"Can't change request for an archived release")
return

if action is UpdateRequest.stable:
if action in (UpdateRequest.stable, UpdateRequest.batched):
settings = request.registry.settings
result, reason = update.check_requirements(request.db, settings)
if not result:
Expand Down
6 changes: 4 additions & 2 deletions bodhi/server/templates/update.html
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@
$("#updatebuttons #edit").attr('href',
'${request.route_url("update_edit", id=update.alias)}');

$.each(['testing', 'stable', 'unpush', 'revoke'], function(i, action) {
$.each(['testing', 'stable', 'batched', 'unpush', 'revoke'], function(i, action) {
$("#updatebuttons #" + action).click(function() {
$("#updatebuttons a").addClass('disabled');
cabbage.spin(); // for real!
Expand Down Expand Up @@ -411,8 +411,10 @@ <h1>
<a id='testing' class="btn btn-sm btn-success"><span class="fa fa-fw fa-arrow-circle-right"></span> Push to Testing</a>
% endif
% if update.status.description not in ['stable', 'obsolete']:
<a id='stable' class="btn btn-sm btn-success"><span class="fa fa-fw fa-arrow-circle-right"></span> Push to Stable</a>
<a id='batched' class="btn btn-sm btn-success"><span class="fa fa-fw fa-arrow-circle-right"></span> Push to Batched</a>
% endif
% elif update.request.description == 'batched':
<a id='stable' class="btn btn-sm btn-success"><span class="fa fa-fw fa-arrow-circle-right"></span> Push to Stable</a>
% else:
<a id='revoke' class="btn btn-sm btn-danger"><span class="fa fa-fw fa-arrow-circle-left"></span> Revoke</a>
% endif
Expand Down
1 change: 1 addition & 0 deletions bodhi/server/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,7 @@ def request2html(context, request):
'obsolete': 'default',
'testing': 'warning',
'stable': 'success',
'batched': 'success',
}.get(request)

return "<span class='label label-%s'>%s</span>" % (cls, request)
Expand Down
2 changes: 1 addition & 1 deletion bodhi/server/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,7 @@ def validate_request(request):
if 'request' not in request.validated:
# Invalid request. Let the colander error from our schemas.py bubble up.
return
if request.validated['request'] is UpdateRequest.stable:
if request.validated['request'] in (UpdateRequest.stable, UpdateRequest.batched):
target = UpdateStatus.stable
elif request.validated['request'] is UpdateRequest.testing:
target = UpdateStatus.testing
Expand Down
2 changes: 1 addition & 1 deletion bodhi/tests/server/consumers/test_masher.py
Original file line number Diff line number Diff line change
Expand Up @@ -1053,7 +1053,7 @@ def test_stable_requirements_met_during_push(self, *args):

# Ensure the masher set the autokarma once the push is done
self.assertEquals(up.locked, False)
self.assertEquals(up.request, UpdateRequest.stable)
self.assertEquals(up.request, UpdateRequest.batched)

@mock.patch(**mock_taskotron_results)
@mock.patch('bodhi.server.consumers.masher.MasherThread.update_comps')
Expand Down
65 changes: 65 additions & 0 deletions bodhi/tests/server/scripts/test_dequeue_stable.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# -*- coding: utf-8 -*-
# Copyright © 2017 Caleigh Runge-Hottman
#
# This file is part of Bodhi.
#
# This program is free software; you can redistribute it and/or
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, let's put a Copyright claim here.

# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
"""
This module contains tests for the bodhi.server.scripts.dequeue_stable module.
"""
from datetime import datetime, timedelta

from click import testing

from bodhi.server import models
from bodhi.server.scripts import dequeue_stable
from bodhi.tests.server.base import BaseTestCase


class TestDequeueStable(BaseTestCase):
"""
This class contains tests for the dequeue_stable() function.
"""
def test_dequeue_stable(self):
"""
Assert that dequeue_stable moves only the batched updates to stable.
"""
runner = testing.CliRunner()

update = self.db.query(models.Update).all()[0]
update.request = models.UpdateRequest.batched
update.locked = False
update.date_testing = datetime.utcnow() - timedelta(days=7)
self.db.commit()

result = runner.invoke(dequeue_stable.dequeue_stable, [])
self.assertEqual(result.exit_code, 0)

update = self.db.query(models.Update).all()[0]
self.assertEqual(update.request, models.UpdateRequest.stable)

def test_dequeue_stable_exception(self):
"""
Assert that a locked update triggers an exception, and doesn't move to stable.
"""
runner = testing.CliRunner()
update = self.db.query(models.Update).all()[0]
update.request = models.UpdateRequest.batched
self.db.commit()

result = runner.invoke(dequeue_stable.dequeue_stable, [])

self.assertEqual(result.exit_code, 1)
self.assertEqual(result.output, u"Can't change the request on a locked update\n")
Loading