Skip to content

Commit bfeb6d8

Browse files
humitosagjohnson
andauthored
Build: fail builds without configuration file or using v1 (#10355)
* Config: deprecated notification for builds without config file When we detect a build is built without a Read the Docs configuration file (`.readthedocs.yaml`) we show multiple notifications: - a static warning message in the build detail's page - a persistent on-site notification to all maintainers/admin of the project - send a weekly email (at most) This is the initial step to attempt making users to migrate to our config file v2, giving them a enough window to do this and avoid breaking their builds in the future. Closes #10348 * Test: invert logic * Build: fail builds without configuration file or using v1 Use a feature flag to decide whether or not hard fail the builds that are not using a configuration file at all or are using v1. This will be useful in the future when we want to make the builds to fail during a reduced period of time to inform users/customers about this deprecation. Related #10351 * Update copy to follow the same style * Upgrade `common/` to use the latest versions for pre-commit This is an attempt to solve the issue with the CircleCI `checks` test. * Dont trigger the task on each build, and linting * Linting * Lint * Latest `common/` * Apply suggestions from code review Co-authored-by: Anthony <[email protected]> --------- Co-authored-by: Anthony <[email protected]>
1 parent e8f4887 commit bfeb6d8

File tree

5 files changed

+30
-13
lines changed

5 files changed

+30
-13
lines changed

readthedocs/doc_builder/director.py

+6
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,12 @@ def checkout(self):
240240
self.data.build["config"] = self.data.config.as_dict()
241241
self.data.build["readthedocs_yaml_path"] = custom_config_file
242242

243+
# Raise a build error if the project is not using a config file or using v1
244+
if self.data.project.has_feature(
245+
Feature.NO_CONFIG_FILE_DEPRECATED
246+
) and self.data.config.version not in ("2", 2):
247+
raise BuildUserError(BuildUserError.NO_CONFIG_FILE_DEPRECATED)
248+
243249
if self.vcs_repository.supports_submodules:
244250
self.vcs_repository.update_submodules(self.data.config)
245251

readthedocs/doc_builder/exceptions.py

+5
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ class BuildUserError(BuildBaseException):
5757
"Ensure your project is configured to use the output path "
5858
"'$READTHEDOCS_OUTPUT/html' instead."
5959
)
60+
NO_CONFIG_FILE_DEPRECATED = gettext_noop(
61+
"The configuration file required to build documentation is missing from your project. "
62+
"Add a configuration file to your project to make it build successfully. "
63+
"Read more at https://docs.readthedocs.io/en/stable/config-file/v2.html"
64+
)
6065

6166

6267
class BuildUserSkip(BuildUserError):

readthedocs/projects/models.py

+14-7
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ class ProjectRelationship(models.Model):
107107
def __str__(self):
108108
return '{} -> {}'.format(self.parent, self.child)
109109

110-
def save(self, *args, **kwargs): # pylint: disable=arguments-differ
110+
def save(self, *args, **kwargs):
111111
if not self.alias:
112112
self.alias = self.child.slug
113113
super().save(*args, **kwargs)
@@ -544,12 +544,14 @@ class Meta:
544544
def __str__(self):
545545
return self.name
546546

547-
def save(self, *args, **kwargs): # pylint: disable=arguments-differ
547+
def save(self, *args, **kwargs):
548548
if not self.slug:
549549
# Subdomains can't have underscores in them.
550550
self.slug = slugify(self.name)
551551
if not self.slug:
552-
raise Exception(_('Model must have slug'))
552+
raise Exception( # pylint: disable=broad-exception-raised
553+
_("Model must have slug")
554+
)
553555
super().save(*args, **kwargs)
554556

555557
try:
@@ -567,7 +569,7 @@ def save(self, *args, **kwargs): # pylint: disable=arguments-differ
567569
)
568570
self.versions.filter(slug=LATEST).update(identifier=self.default_branch)
569571

570-
def delete(self, *args, **kwargs): # pylint: disable=arguments-differ
572+
def delete(self, *args, **kwargs):
571573
from readthedocs.projects.tasks.utils import clean_project_resources
572574

573575
# Remove extra resources
@@ -1747,7 +1749,7 @@ class Domain(TimeStampedModel):
17471749
canonical = models.BooleanField(
17481750
default=False,
17491751
help_text=_(
1750-
"This domain is the primary one where the documentation is " "served from",
1752+
"This domain is the primary one where the documentation is served from",
17511753
),
17521754
)
17531755
https = models.BooleanField(
@@ -1830,7 +1832,7 @@ def restart_validation_process(self):
18301832
self.validation_process_start = timezone.now()
18311833
self.save()
18321834

1833-
def save(self, *args, **kwargs): # pylint: disable=arguments-differ
1835+
def save(self, *args, **kwargs):
18341836
parsed = urlparse(self.domain)
18351837
if parsed.scheme or parsed.netloc:
18361838
self.domain = parsed.netloc
@@ -1949,6 +1951,7 @@ def add_features(sender, **kwargs):
19491951
DONT_CREATE_INDEX = "dont_create_index"
19501952
USE_RCLONE = "use_rclone"
19511953
HOSTING_INTEGRATIONS = "hosting_integrations"
1954+
NO_CONFIG_FILE_DEPRECATED = "no_config_file"
19521955

19531956
FEATURES = (
19541957
(ALLOW_DEPRECATED_WEBHOOKS, _("Webhook: Allow deprecated webhook views")),
@@ -2139,6 +2142,10 @@ def add_features(sender, **kwargs):
21392142
"Proxito: Inject 'readthedocs-client.js' as <script> HTML tag in responses."
21402143
),
21412144
),
2145+
(
2146+
NO_CONFIG_FILE_DEPRECATED,
2147+
_("Build: Building without a configuration file is deprecated."),
2148+
),
21422149
)
21432150

21442151
FEATURES = sorted(FEATURES, key=lambda l: l[1])
@@ -2210,6 +2217,6 @@ class EnvironmentVariable(TimeStampedModel, models.Model):
22102217
def __str__(self):
22112218
return self.name
22122219

2213-
def save(self, *args, **kwargs): # pylint: disable=arguments-differ
2220+
def save(self, *args, **kwargs):
22142221
self.value = quote(self.value)
22152222
return super().save(*args, **kwargs)

readthedocs/projects/tasks/builds.py

+4-5
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ class TaskData:
107107
api_client: API = None
108108

109109
start_time: timezone.datetime = None
110-
# pylint: disable=unsubscriptable-object
111110
environment_class: type[DockerBuildEnvironment] | type[LocalBuildEnvironment] = None
112111
build_director: BuildDirector = None
113112
config: BuildConfigV1 | BuildConfigV2 = None
@@ -886,7 +885,7 @@ def store_build_artifacts(self):
886885
build_media_storage.rclone_sync_directory(from_path, to_path)
887886
else:
888887
build_media_storage.sync_directory(from_path, to_path)
889-
except Exception:
888+
except Exception as exc:
890889
# NOTE: the exceptions reported so far are:
891890
# - botocore.exceptions:HTTPClientError
892891
# - botocore.exceptions:ClientError
@@ -900,7 +899,7 @@ def store_build_artifacts(self):
900899
# Re-raise the exception to fail the build and handle it
901900
# automatically at `on_failure`.
902901
# It will clearly communicate the error to the user.
903-
raise BuildAppError("Error uploading files to the storage.")
902+
raise BuildAppError("Error uploading files to the storage.") from exc
904903

905904
# Delete formats
906905
for media_type in types_to_delete:
@@ -912,7 +911,7 @@ def store_build_artifacts(self):
912911
)
913912
try:
914913
build_media_storage.delete_directory(media_path)
915-
except Exception:
914+
except Exception as exc:
916915
# NOTE: I didn't find any log line for this case yet
917916
log.exception(
918917
"Error deleting files from storage",
@@ -922,7 +921,7 @@ def store_build_artifacts(self):
922921
# Re-raise the exception to fail the build and handle it
923922
# automatically at `on_failure`.
924923
# It will clearly communicate the error to the user.
925-
raise BuildAppError("Error deleting files from storage.")
924+
raise BuildAppError("Error deleting files from storage.") from exc
926925

927926
log.info(
928927
"Store build artifacts finished.",

0 commit comments

Comments
 (0)