Skip to content

Remove asserts from code. #5452

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

Merged
merged 1 commit into from
Mar 20, 2019
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
3 changes: 2 additions & 1 deletion readthedocs/builds/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ def commit_name(self):
return self.identifier

# By now we must have handled all special versions.
assert self.slug not in NON_REPOSITORY_VERSIONS
if self.slug in NON_REPOSITORY_VERSIONS:
raise Exception('All special versions must be handled by now.')

if self.type in (BRANCH, TAG):
# If this version is a branch or a tag, the verbose_name will
Expand Down
6 changes: 3 additions & 3 deletions readthedocs/builds/version_slug.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,9 @@ def create_slug(self, model_instance):
kwargs[self.attname] = slug
count += 1

assert self.test_pattern.match(slug), (
'Invalid generated slug: {slug}'.format(slug=slug)
)
is_slug_valid = self.test_pattern.match(slug)
if not is_slug_valid:
raise Exception('Invalid generated slug: {slug}'.format(slug=slug))
return slug

def pre_save(self, model_instance, add):
Expand Down
4 changes: 3 additions & 1 deletion readthedocs/core/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

from readthedocs.builds.constants import LATEST, BUILD_STATE_TRIGGERED
from readthedocs.doc_builder.constants import DOCKER_LIMITS
from readthedocs.projects.exceptions import InvalidParamsException

log = logging.getLogger(__name__)

Expand All @@ -33,7 +34,8 @@ def broadcast(type, task, args, kwargs=None, callback=None): # pylint: disable=
`callback` should be a task signature that will be run once,
after all of the broadcast tasks have finished running.
"""
assert type in ['web', 'app', 'build']
if type not in ['web', 'app', 'build']:
raise InvalidParamsException('allowed value of `type` are web, app and build.')
if kwargs is None:
kwargs = {}
default_queue = getattr(settings, 'CELERY_DEFAULT_QUEUE', 'celery')
Expand Down
6 changes: 4 additions & 2 deletions readthedocs/doc_builder/environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ def __init__(
self.cwd = cwd
self.environment = os.environ.copy()
if environment is not None:
assert 'PATH' not in environment, "PATH can't be set"
if 'PATH' in environment:
raise BuildEnvironmentError('\'PATH\' can\'t be set.')
self.environment.update(environment)

self.combine_output = combine_output
Expand Down Expand Up @@ -434,7 +435,8 @@ def run_command_class(
env_path = self.environment.pop('BIN_PATH', None)
if 'bin_path' not in kwargs and env_path:
kwargs['bin_path'] = env_path
assert 'environment' not in kwargs, "environment can't be passed in via commands."
if 'environment' in kwargs:
raise BuildEnvironmentError('environment can\'t be passed in via commands.')
kwargs['environment'] = self.environment

# ``build_env`` is passed as ``kwargs`` when it's called from a
Expand Down
7 changes: 7 additions & 0 deletions readthedocs/projects/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,10 @@ class ProjectSpamError(Exception):
This error is not raised to users, we use this for banning users in the
background.
"""


class InvalidParamsException(Exception):

"""Error raised when incorrect parameters are passed to a function/class."""
Copy link
Member

Choose a reason for hiding this comment

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

We can use ValueError and TypeError in most of the cases of this custom Exception I think, feel free to create another PR with this changes

https://docs.python.org/3/library/exceptions.html#ValueError
https://docs.python.org/3/library/exceptions.html#TypeError

Copy link
Member Author

Choose a reason for hiding this comment

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

@stsewd
Thanks for this.
I'll be opening another PR about this soon.


pass
5 changes: 3 additions & 2 deletions readthedocs/projects/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
from readthedocs.worker import app

from .constants import LOG_TEMPLATE
from .exceptions import ProjectConfigurationError, RepositoryError
from .exceptions import ProjectConfigurationError, RepositoryError, InvalidParamsException
from .models import Domain, HTMLFile, ImportedFile, Project
from .signals import (
after_build,
Expand Down Expand Up @@ -101,7 +101,8 @@ def get_version(project=None, version_pk=None):
:returns: a data-complete version object
:rtype: builds.models.APIVersion
"""
assert (project or version_pk), 'project or version_pk is needed'
if not (project or version_pk):
raise InvalidParamsException('project or version_pk is needed')
if version_pk:
version_data = api_v2.version(version_pk).get()
else:
Expand Down
10 changes: 6 additions & 4 deletions readthedocs/search/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from django_elasticsearch_dsl.registries import registry

from readthedocs.worker import app
from readthedocs.projects.exceptions import InvalidParamsException
from .utils import _get_index, _get_document

log = logging.getLogger(__name__)
Expand All @@ -14,10 +15,11 @@ def index_objects_to_es(
app_label, model_name, document_class, index_name=None, chunk=None, objects_id=None
):

assert not (chunk and objects_id), \
"You can not pass both chunk and objects_id"
assert (chunk or objects_id), \
"You must pass a chunk or objects_id"
if chunk and objects_id:
raise InvalidParamsException('You can not pass both chunk and objects_id.')

if not (chunk or objects_id):
raise InvalidParamsException('You must pass a chunk or objects_id.')

model = apps.get_model(app_label, model_name)
document = _get_document(model=model, document_class=document_class)
Expand Down