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

Build: fail builds without configuration file or using v1 #10355

Merged
merged 12 commits into from
Jun 14, 2023
2 changes: 1 addition & 1 deletion common
6 changes: 6 additions & 0 deletions readthedocs/doc_builder/director.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,12 @@ def checkout(self):
self.data.build["config"] = self.data.config.as_dict()
self.data.build["readthedocs_yaml_path"] = custom_config_file

# Raise a build error if the project is not using a config file or using v1
if self.data.project.has_feature(
Feature.NO_CONFIG_FILE_DEPRECATED
) and self.data.config.version not in ("2", 2):
raise BuildUserError(BuildUserError.NO_CONFIG_FILE_DEPRECATED)

if self.vcs_repository.supports_submodules:
self.vcs_repository.update_submodules(self.data.config)

Expand Down
5 changes: 5 additions & 0 deletions readthedocs/doc_builder/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ class BuildUserError(BuildBaseException):
"Ensure your project is configured to use the output path "
"'$READTHEDOCS_OUTPUT/html' instead."
)
NO_CONFIG_FILE_DEPRECATED = gettext_noop(
"Your project doesn't have the required configuration file to build the documentation. "
"Add a configuration file to your project to make it build successfully. "
"Read more at https://docs.readthedocs.io/en/stable/config-file/v2.html"
humitos marked this conversation as resolved.
Show resolved Hide resolved
)


class BuildUserSkip(BuildUserError):
Expand Down
21 changes: 14 additions & 7 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class ProjectRelationship(models.Model):
def __str__(self):
return '{} -> {}'.format(self.parent, self.child)

def save(self, *args, **kwargs): # pylint: disable=arguments-differ
def save(self, *args, **kwargs):
if not self.alias:
self.alias = self.child.slug
super().save(*args, **kwargs)
Expand Down Expand Up @@ -544,12 +544,14 @@ class Meta:
def __str__(self):
return self.name

def save(self, *args, **kwargs): # pylint: disable=arguments-differ
def save(self, *args, **kwargs):
if not self.slug:
# Subdomains can't have underscores in them.
self.slug = slugify(self.name)
if not self.slug:
raise Exception(_('Model must have slug'))
raise Exception( # pylint: disable=broad-exception-raised
_("Model must have slug")
)
super().save(*args, **kwargs)

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

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

# Remove extra resources
Expand Down Expand Up @@ -1747,7 +1749,7 @@ class Domain(TimeStampedModel):
canonical = models.BooleanField(
default=False,
help_text=_(
"This domain is the primary one where the documentation is " "served from",
"This domain is the primary one where the documentation is served from",
),
)
https = models.BooleanField(
Expand Down Expand Up @@ -1830,7 +1832,7 @@ def restart_validation_process(self):
self.validation_process_start = timezone.now()
self.save()

def save(self, *args, **kwargs): # pylint: disable=arguments-differ
def save(self, *args, **kwargs):
parsed = urlparse(self.domain)
if parsed.scheme or parsed.netloc:
self.domain = parsed.netloc
Expand Down Expand Up @@ -1949,6 +1951,7 @@ def add_features(sender, **kwargs):
DONT_CREATE_INDEX = "dont_create_index"
USE_RCLONE = "use_rclone"
HOSTING_INTEGRATIONS = "hosting_integrations"
NO_CONFIG_FILE_DEPRECATED = "no_config_file"

FEATURES = (
(ALLOW_DEPRECATED_WEBHOOKS, _("Webhook: Allow deprecated webhook views")),
Expand Down Expand Up @@ -2139,6 +2142,10 @@ def add_features(sender, **kwargs):
"Proxito: Inject 'readthedocs-client.js' as <script> HTML tag in responses."
),
),
(
NO_CONFIG_FILE_DEPRECATED,
_("Build: Building without a configuration file is deprecated."),
),
)

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

def save(self, *args, **kwargs): # pylint: disable=arguments-differ
def save(self, *args, **kwargs):
self.value = quote(self.value)
return super().save(*args, **kwargs)
9 changes: 4 additions & 5 deletions readthedocs/projects/tasks/builds.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ class TaskData:
api_client: API = None

start_time: timezone.datetime = None
# pylint: disable=unsubscriptable-object
environment_class: type[DockerBuildEnvironment] | type[LocalBuildEnvironment] = None
build_director: BuildDirector = None
config: BuildConfigV1 | BuildConfigV2 = None
Expand Down Expand Up @@ -886,7 +885,7 @@ def store_build_artifacts(self):
build_media_storage.rclone_sync_directory(from_path, to_path)
else:
build_media_storage.sync_directory(from_path, to_path)
except Exception:
except Exception as exc:
# NOTE: the exceptions reported so far are:
# - botocore.exceptions:HTTPClientError
# - botocore.exceptions:ClientError
Expand All @@ -900,7 +899,7 @@ def store_build_artifacts(self):
# Re-raise the exception to fail the build and handle it
# automatically at `on_failure`.
# It will clearly communicate the error to the user.
raise BuildAppError("Error uploading files to the storage.")
raise BuildAppError("Error uploading files to the storage.") from exc
humitos marked this conversation as resolved.
Show resolved Hide resolved

# Delete formats
for media_type in types_to_delete:
Expand All @@ -912,7 +911,7 @@ def store_build_artifacts(self):
)
try:
build_media_storage.delete_directory(media_path)
except Exception:
except Exception as exc:
# NOTE: I didn't find any log line for this case yet
log.exception(
"Error deleting files from storage",
Expand All @@ -922,7 +921,7 @@ def store_build_artifacts(self):
# Re-raise the exception to fail the build and handle it
# automatically at `on_failure`.
# It will clearly communicate the error to the user.
raise BuildAppError("Error deleting files from storage.")
raise BuildAppError("Error deleting files from storage.") from exc

log.info(
"Store build artifacts finished.",
Expand Down