Skip to content

Commit ee3a3a9

Browse files
humitoshumitos
committed
Build: add a new "Cancelled" final state
This commit introduces a new build final state: "Cancelled". This state is shown in the build listing page and also in the build details page. The build will end up on this state when: - The user cancels the build by clicking on the button - The build is cancelled due to inactivity - The build is cancelled because it was duplicated Ther are a lot of changes required for this: - HTML template logic for build details and listing pages - API changes to add a new value - Knockout.js logic for build details page - `Build` model changes to support a new state - Update different places of core logic to change checking for `finished` state to `BUILD_FINAL_STATES` instead - Documentation changes to mention the new state - Celery build handler `after_return` to decide if it's `finished` or `cancelled` the ending state - Logic to decide if the build should be killed due to inactivity Reference #9100
1 parent 7db6582 commit ee3a3a9

File tree

23 files changed

+161
-95
lines changed

23 files changed

+161
-95
lines changed

‎docs/user/api/v3.rst

+1-1
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,7 @@ Build details
694694
:>json string created: The ISO-8601 datetime when the build was created.
695695
:>json string finished: The ISO-8601 datetime when the build has finished.
696696
:>json integer duration: The length of the build in seconds.
697-
:>json string state: The state of the build (one of ``triggered``, ``building``, ``installing``, ``cloning``, or ``finished``)
697+
:>json string state: The state of the build (one of ``triggered``, ``building``, ``installing``, ``cloning``, ``finished`` or ``cancelled``)
698698
:>json string error: An error message if the build was unsuccessful
699699

700700
:query string expand: allows to add/expand some extra fields in the response.

‎media/javascript/build_updater.js

+15-11
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,23 @@
3030
var el = $(this.buildDiv + ' span#build-' + prop);
3131

3232
if (prop == 'success') {
33-
if (data.hasOwnProperty('state') && data['state'] != 'finished') {
33+
if (data.hasOwnProperty('state') && data['state'] != 'finished') && data['state'] != 'cancelled' {
3434
val = "Not yet finished";
3535
}
3636
else {
37+
// TODO: I'm not sure what to do with these. We are
38+
// adding a third option here ("Cancelled") that's not
39+
// "Passed" nor "Failed". There are many other places in
40+
// the code where we are assuming only two possible
41+
// options.
3742
val = val ? "Passed" : "Failed";
3843
}
3944
}
4045

4146
if (prop == 'state') {
4247
val = val.charAt(0).toUpperCase() + val.slice(1);
4348

44-
if (val == 'Finished') {
49+
if (val == 'Finished' || val == 'Cancelled') {
4550
_this.stopPolling();
4651
}
4752
}
@@ -55,26 +60,26 @@
5560

5661
BuildUpdater.prototype.getBuild = function() {
5762
_this = this;
58-
63+
5964
$.get(this.buildUrl, function(data) {
6065
_this.render(data);
6166
});
6267
};
6368

64-
// If the build with ID `this.buildId` has a state other than finished, poll
65-
// the server every 5 seconds for the current status. Update the details
66-
// page with the latest values from the server, to keep the user informed of
67-
// progress.
69+
// If the build with ID `this.buildId` has a state other than finished or
70+
// cancelled, poll the server every 5 seconds for the current status. Update
71+
// the details page with the latest values from the server, to keep the user
72+
// informed of progress.
6873
//
69-
// If we haven't received a 'finished' state back the server in 10 minutes,
74+
// If we haven't received a 'finished'/'cancelled' state back the server in 10 minutes,
7075
// stop polling.
7176
BuildUpdater.prototype.startPolling = function() {
7277
var stateSpan = $(this.buildDiv + ' span#build-state');
7378
var _this = this;
7479

7580
// If the build is already finished, or it isn't displayed on the page,
7681
// ignore it.
77-
if (stateSpan.text() == 'Finished' || stateSpan.length === 0) {
82+
if (stateSpan.text() == 'Finished' || stateSpan.text() == 'Cancelled' || stateSpan.length === 0) {
7883
return;
7984
}
8085

@@ -117,7 +122,7 @@
117122
if (prop == 'state') {
118123
// Show the success value ("Passed" or "Failed") if the build
119124
// finished. Otherwise, show the state value.
120-
if (val == 'Finished') {
125+
if (val == 'Finished' || val == 'Cancelled') {
121126
val = data['success'];
122127
_this.stopPolling();
123128
} else {
@@ -134,4 +139,3 @@
134139

135140

136141
}).call(this);
137-

‎readthedocs/api/v2/templates/restapi/log.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ Version: {{ build.version_slug }}
55
Commit: {{ build.commit }}
66
Date: {{ build.date }}
77
State: {{ build.state }}
8-
Success: {% if build.state == 'finished' %}{{ build.success }}{% else %}Unknown{% endif %}
8+
Success: {% if build.state == 'finished' or build.state == 'cancelled' %}{{ build.success }}{% else %}Unknown{% endif %}
99

1010
{% for command in build.commands %}
1111
[rtd-command-info] start-time: {{ command.start_time }}, end-time: {{ command.end_time }}, duration: {{ command.run_time }}, exit-code: {{ command.exit_code }}

‎readthedocs/api/v3/filters.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import django_filters.rest_framework as filters
22

3-
from readthedocs.builds.constants import BUILD_STATE_FINISHED
3+
from readthedocs.builds.constants import BUILD_FINAL_STATES
44
from readthedocs.builds.models import Build, Version
5-
from readthedocs.oauth.models import RemoteRepository, RemoteOrganization
5+
from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
66
from readthedocs.projects.models import Project
77

88

@@ -45,9 +45,9 @@ class Meta:
4545

4646
def get_running(self, queryset, name, value):
4747
if value:
48-
return queryset.exclude(state=BUILD_STATE_FINISHED)
48+
return queryset.exclude(state__in=BUILD_FINAL_STATES)
4949

50-
return queryset.filter(state=BUILD_STATE_FINISHED)
50+
return queryset.filter(state__in=BUILD_FINAL_STATES)
5151

5252

5353
class RemoteRepositoryFilter(filters.FilterSet):

‎readthedocs/builds/constants.py

+19-12
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,27 @@
33
from django.conf import settings
44
from django.utils.translation import gettext_lazy as _
55

6-
BUILD_STATE_TRIGGERED = 'triggered'
7-
BUILD_STATE_CLONING = 'cloning'
8-
BUILD_STATE_INSTALLING = 'installing'
9-
BUILD_STATE_BUILDING = 'building'
10-
BUILD_STATE_UPLOADING = 'uploading'
11-
BUILD_STATE_FINISHED = 'finished'
6+
BUILD_STATE_TRIGGERED = "triggered"
7+
BUILD_STATE_CLONING = "cloning"
8+
BUILD_STATE_INSTALLING = "installing"
9+
BUILD_STATE_BUILDING = "building"
10+
BUILD_STATE_UPLOADING = "uploading"
11+
BUILD_STATE_FINISHED = "finished"
12+
BUILD_STATE_CANCELLED = "cancelled"
1213

1314
BUILD_STATE = (
14-
(BUILD_STATE_TRIGGERED, _('Triggered')),
15-
(BUILD_STATE_CLONING, _('Cloning')),
16-
(BUILD_STATE_INSTALLING, _('Installing')),
17-
(BUILD_STATE_BUILDING, _('Building')),
18-
(BUILD_STATE_UPLOADING, _('Uploading')),
19-
(BUILD_STATE_FINISHED, _('Finished')),
15+
(BUILD_STATE_TRIGGERED, _("Triggered")),
16+
(BUILD_STATE_CLONING, _("Cloning")),
17+
(BUILD_STATE_INSTALLING, _("Installing")),
18+
(BUILD_STATE_BUILDING, _("Building")),
19+
(BUILD_STATE_UPLOADING, _("Uploading")),
20+
(BUILD_STATE_FINISHED, _("Finished")),
21+
(BUILD_STATE_CANCELLED, _("Cancelled")),
22+
)
23+
24+
BUILD_FINAL_STATES = (
25+
BUILD_STATE_FINISHED,
26+
BUILD_STATE_CANCELLED,
2027
)
2128

2229
BUILD_TYPES = (

‎readthedocs/builds/filters.py

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
import structlog
2-
32
from django.forms.widgets import HiddenInput
43
from django.utils.translation import gettext_lazy as _
54
from django_filters import CharFilter, ChoiceFilter, FilterSet
65

7-
from readthedocs.builds.constants import BUILD_STATE_FINISHED
6+
from readthedocs.builds.constants import BUILD_FINAL_STATES, BUILD_STATE_FINISHED
87

98
log = structlog.get_logger(__name__)
109

@@ -37,7 +36,7 @@ def get_state(self, queryset, name, value):
3736
queryset = queryset.filter(state=BUILD_STATE_FINISHED, success=True)
3837
elif value == self.STATE_FAILED:
3938
queryset = queryset.filter(
40-
state=BUILD_STATE_FINISHED,
39+
state__in=BUILD_FINAL_STATES,
4140
success=False,
4241
)
4342
return queryset

‎readthedocs/builds/migrations/0043_add_cancelled_state.py

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# Generated by Django 3.2.13 on 2022-05-04 11:03
2+
3+
from django.db import migrations, models
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
("builds", "0042_version_state"),
10+
]
11+
12+
operations = [
13+
migrations.AlterField(
14+
model_name="build",
15+
name="state",
16+
field=models.CharField(
17+
choices=[
18+
("triggered", "Triggered"),
19+
("cloning", "Cloning"),
20+
("installing", "Installing"),
21+
("building", "Building"),
22+
("uploading", "Uploading"),
23+
("finished", "Finished"),
24+
("cancelled", "Cancelled"),
25+
],
26+
db_index=True,
27+
default="finished",
28+
max_length=55,
29+
verbose_name="State",
30+
),
31+
),
32+
]

‎readthedocs/builds/models.py

+3-2
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import readthedocs.builds.automation_actions as actions
2222
from readthedocs.builds.constants import (
2323
BRANCH,
24+
BUILD_FINAL_STATES,
2425
BUILD_STATE,
2526
BUILD_STATE_FINISHED,
2627
BUILD_STATE_TRIGGERED,
@@ -933,8 +934,8 @@ def get_commit_url(self):
933934

934935
@property
935936
def finished(self):
936-
"""Return if build has a finished state."""
937-
return self.state == BUILD_STATE_FINISHED
937+
"""Return if build has an end state."""
938+
return self.state in BUILD_FINAL_STATES
938939

939940
@property
940941
def is_stale(self):

‎readthedocs/builds/querysets.py

+14-4
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
"""Build and Version QuerySet classes."""
22
import datetime
3-
import structlog
43

4+
import structlog
55
from django.db import models
66
from django.db.models import Q
77
from django.utils import timezone
88

99
from readthedocs.builds.constants import (
10+
BUILD_STATE_CANCELLED,
1011
BUILD_STATE_FINISHED,
1112
BUILD_STATE_TRIGGERED,
1213
EXTERNAL,
@@ -225,9 +226,18 @@ def concurrent(self, project):
225226
query |= Q(project__in=organization.projects.all())
226227

227228
concurrent = (
228-
self.filter(query)
229-
.exclude(state__in=[BUILD_STATE_TRIGGERED, BUILD_STATE_FINISHED])
230-
).distinct().count()
229+
(
230+
self.filter(query).exclude(
231+
state__in=[
232+
BUILD_STATE_TRIGGERED,
233+
BUILD_STATE_FINISHED,
234+
BUILD_STATE_CANCELLED,
235+
]
236+
)
237+
)
238+
.distinct()
239+
.count()
240+
)
231241

232242
max_concurrent = Project.objects.max_concurrent_builds(project)
233243
log.info(

‎readthedocs/builds/static-src/builds/js/detail.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,11 @@ function BuildDetailView(instance) {
3232
/* Instance variables */
3333
self.state = ko.observable(instance.state);
3434
self.state_display = ko.observable(instance.state_display);
35+
self.cancelled = ko.computed(function () {
36+
return self.state() === 'cancelled';
37+
});
3538
self.finished = ko.computed(function () {
36-
return self.state() === 'finished';
39+
return self.state() === 'finished' || self.state() === 'cancelled';
3740
});
3841
self.date = ko.observable(instance.date);
3942
self.success = ko.observable(instance.success);

‎readthedocs/builds/static/builds/css/detail.css

+4-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ div.build-detail div.build-state {
1919
}
2020

2121
div.build-detail div.build-state > span.build-state-successful,
22-
div.build-detail div.build-state > span.build-state-failed {
22+
div.build-detail div.build-state > span.build-state-failed,
23+
div.build-detail div.build-state > span.build-state-cancelled {
2324
padding: .3em .5em;
2425
border-radius: .3em;
2526
-moz-border-radius: .3em;
@@ -28,7 +29,8 @@ div.build-detail div.build-state > span.build-state-failed {
2829
color: white;
2930
}
3031

31-
div.build-detail div.build-state > span.build-state-failed {
32+
div.build-detail div.build-state > span.build-state-failed,
33+
div.build-detail div.build-state > span.build-state-cancelled {
3234
background: #a55;
3335
}
3436
div.build-detail div.build-state > span.build-state-successful {

‎readthedocs/builds/static/builds/js/detail.js

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎readthedocs/builds/tests/test_build_queryset.py

+9-12
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
1-
import pytest
2-
31
import django_dynamic_fixture as fixture
4-
from django.conf import settings
2+
import pytest
53

6-
from readthedocs.builds.querysets import BuildQuerySet
7-
from readthedocs.builds.models import Build, Version
4+
from readthedocs.builds.models import Build
85
from readthedocs.organizations.models import Organization
9-
from readthedocs.projects.models import Project, Feature
6+
from readthedocs.projects.models import Project
107

118

129
@pytest.mark.django_db
@@ -18,7 +15,7 @@ def test_concurrent_builds(self):
1815
max_concurrent_builds=None,
1916
main_language_project=None,
2017
)
21-
for state in ('triggered', 'building', 'cloning', 'finished'):
18+
for state in ("triggered", "building", "cloning", "finished", "cancelled"):
2219
fixture.get(
2320
Build,
2421
project=project,
@@ -39,7 +36,7 @@ def test_concurrent_builds_project_limited(self):
3936
max_concurrent_builds=2,
4037
main_language_project=None,
4138
)
42-
for state in ('triggered', 'building', 'cloning', 'finished'):
39+
for state in ("triggered", "building", "cloning", "finished", "cancelled"):
4340
fixture.get(
4441
Build,
4542
project=project,
@@ -58,7 +55,7 @@ def test_concurrent_builds_translations(self):
5855
max_concurrent_builds=None,
5956
main_language_project=project,
6057
)
61-
for state in ('triggered', 'building', 'cloning', 'finished'):
58+
for state in ("triggered", "building", "cloning", "finished", "cancelled"):
6259
fixture.get(
6360
Build,
6461
project=project,
@@ -90,7 +87,7 @@ def test_concurrent_builds_organization(self):
9087
organization.projects.add(project)
9188

9289
for project in organization.projects.all():
93-
for state in ('triggered', 'building', 'cloning', 'finished'):
90+
for state in ("triggered", "building", "cloning", "finished", "cancelled"):
9491
fixture.get(
9592
Build,
9693
project=project,
@@ -124,7 +121,7 @@ def test_concurrent_builds_organization_limited(self):
124121
)
125122
organization.projects.add(project_with_builds)
126123
organization.projects.add(project_without_builds)
127-
for state in ('triggered', 'building', 'cloning', 'finished'):
124+
for state in ("triggered", "building", "cloning", "finished", "cancelled"):
128125
fixture.get(
129126
Build,
130127
project=project_with_builds,
@@ -151,7 +148,7 @@ def test_concurrent_builds_organization_and_project_limited(self):
151148
)
152149
organization.projects.add(project_limited)
153150
organization.projects.add(project_not_limited)
154-
for state in ('triggered', 'building', 'cloning', 'finished'):
151+
for state in ("triggered", "building", "cloning", "finished", "cancelled"):
155152
fixture.get(
156153
Build,
157154
project=project_limited,

0 commit comments

Comments
 (0)