Skip to content

Build: skip build based on commands' exit codes #9649

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Nov 14, 2022
Merged

Build: skip build based on commands' exit codes #9649

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
abc767e
Build: skip build based on commands' exit codes
humitos Oct 10, 2022
cd91bf5
Update readthedocs/doc_builder/exceptions.py
humitos Oct 19, 2022
7c110db
Build: send SUCCESS external build status on skipped builds
humitos Nov 8, 2022
f67209c
Docs: skip build based on a condition
humitos Nov 8, 2022
ced9655
Lint: small darker change
humitos Nov 8, 2022
b0c7e6f
Docs: reference fixed
humitos Nov 8, 2022
fc2dfa8
Docs: improve comment on YAML
humitos Nov 8, 2022
f9eecb4
Build: make usage of `status` variable
humitos Nov 8, 2022
0ca2fc6
Apply suggestions from code review
humitos Nov 8, 2022
428f45d
Docs: update code example for skip builds
humitos Nov 8, 2022
6ba8aae
Docs: example showing how to skip the build based on commit message
humitos Nov 8, 2022
111ee35
Docs: refactor "skipping a build" section (#9717)
humitos Nov 10, 2022
a41cbf2
Docs: use `grep` instead of `case`
humitos Nov 14, 2022
dc79701
Docs: small note about "Why 183?"
humitos Nov 14, 2022
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
12 changes: 9 additions & 3 deletions readthedocs/doc_builder/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
# -*- coding: utf-8 -*-

"""Doc build constants."""

import structlog
import re

import structlog
from django.conf import settings


log = structlog.get_logger(__name__)

PDF_RE = re.compile('Output written on (.*?)')
Expand All @@ -30,3 +28,11 @@
DOCKER_OOM_EXIT_CODE = 137

DOCKER_HOSTNAME_MAX_LEN = 64

# Why 183 exit code?
#
# >>> sum(list('skip'.encode('ascii')))
# 439
# >>> 439 % 256
# 183
RTD_SKIP_BUILD_EXIT_CODE = 183
Copy link
Contributor

Choose a reason for hiding this comment

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

I brought up the point that perhaps we want support for more explicit codes here. If so, perhaps we should choose a less arbitrary value here and allocate a block of exit codes.

183 is in a safe range for posix standards at least though:
https://tldp.org/LDP/abs/html/exitcodes.html

But, do we want something like:

  • Exit code 180: skip build immediately
  • Exit code 181: fail build immediately
  • Exit code 182: retry build later?
  • etc

Not to say we're implementing everything now, but if we will have more than one exit code, I'd suggest starting from a value that doesn't seem so arbitrary.

Copy link
Member Author

Choose a reason for hiding this comment

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

183 is in a safe range for posix standards at least though

Yeah, I checked for this as well and I was happy that "skip" was in that range 😄

I brought up the point that perhaps we want support for more explicit codes here

This looks interesting, but right now I'm not seeing a clear usage for other useful exit codes. We can keep thinking about this and see if we find some other useful cases. In any case, we can reserve the range 180-185 and start with 183 for this one; there is no need to make them consecutive.

5 changes: 4 additions & 1 deletion readthedocs/doc_builder/environments.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@
DOCKER_SOCKET,
DOCKER_TIMEOUT_EXIT_CODE,
DOCKER_VERSION,
RTD_SKIP_BUILD_EXIT_CODE,
)
from .exceptions import BuildAppError, BuildUserError
from .exceptions import BuildAppError, BuildUserError, BuildUserSkip

log = structlog.get_logger(__name__)

Expand Down Expand Up @@ -468,6 +469,8 @@ def run_command_class(
project_slug=self.project.slug if self.project else '',
version_slug=self.version.slug if self.version else '',
)
elif build_cmd.exit_code == RTD_SKIP_BUILD_EXIT_CODE:
raise BuildUserSkip()
else:
# TODO: for now, this still outputs a generic error message
# that is the same across all commands. We could improve this
Expand Down
7 changes: 7 additions & 0 deletions readthedocs/doc_builder/exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ class BuildUserError(BuildBaseException):
)


class BuildUserSkip(BuildUserError):
message = gettext_noop(
"This build was cancelled due the magic exit code was returned by a commmand."
)
state = BUILD_STATE_CANCELLED


class ProjectBuildsSkippedError(BuildUserError):
message = gettext_noop('Builds for this project are temporarily disabled')

Expand Down
3 changes: 3 additions & 0 deletions readthedocs/projects/tasks/builds.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
BuildCancelled,
BuildMaxConcurrencyError,
BuildUserError,
BuildUserSkip,
MkDocsYAMLParseError,
ProjectBuildsSkippedError,
YAMLParseError,
Expand Down Expand Up @@ -280,6 +281,7 @@ class UpdateDocsTask(SyncRepositoryMixin, Task):
YAMLParseError,
BuildCancelled,
BuildUserError,
BuildUserSkip,
RepositoryError,
MkDocsYAMLParseError,
ProjectConfigurationError,
Expand All @@ -289,6 +291,7 @@ class UpdateDocsTask(SyncRepositoryMixin, Task):
exceptions_without_notifications = (
BuildCancelled,
BuildMaxConcurrencyError,
BuildUserSkip,
ProjectBuildsSkippedError,
)

Expand Down