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

Unify usage of settings and remove the usage of getattr for settings #5588

Merged
merged 6 commits into from
Apr 25, 2019
Merged
Show file tree
Hide file tree
Changes from 4 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
8 changes: 4 additions & 4 deletions readthedocs/builds/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@
(UNKNOWN, _('Unknown')),
)

LATEST = getattr(settings, 'RTD_LATEST', 'latest')
LATEST_VERBOSE_NAME = getattr(settings, 'RTD_LATEST_VERBOSE_NAME', 'latest')
LATEST = settings.RTD_LATEST
LATEST_VERBOSE_NAME = settings.RTD_LATEST_VERBOSE_NAME

STABLE = getattr(settings, 'RTD_STABLE', 'stable')
STABLE_VERBOSE_NAME = getattr(settings, 'RTD_STABLE_VERBOSE_NAME', 'stable')
STABLE = settings.RTD_STABLE
STABLE_VERBOSE_NAME = settings.RTD_STABLE_VERBOSE_NAME

# Those names are specialcased version names. They do not correspond to
# branches/tags in a project's repository.
Expand Down
11 changes: 5 additions & 6 deletions readthedocs/builds/syncers.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
local machine.
"""

import getpass
import logging
import os
import shutil
Expand Down Expand Up @@ -64,8 +63,8 @@ def copy(cls, path, target, is_file=False, **kwargs):

Respects the ``MULTIPLE_APP_SERVERS`` setting when copying.
"""
sync_user = getattr(settings, 'SYNC_USER', getpass.getuser())
app_servers = getattr(settings, 'MULTIPLE_APP_SERVERS', [])
sync_user = settings.SYNC_USER
app_servers = settings.MULTIPLE_APP_SERVERS
if app_servers:
log.info('Remote Copy %s to %s on %s', path, target, app_servers)
for server in app_servers:
Expand Down Expand Up @@ -103,8 +102,8 @@ def copy(cls, path, target, host, is_file=False, **kwargs): # pylint: disable=a

Respects the ``MULTIPLE_APP_SERVERS`` setting when copying.
"""
sync_user = getattr(settings, 'SYNC_USER', getpass.getuser())
app_servers = getattr(settings, 'MULTIPLE_APP_SERVERS', [])
sync_user = settings.SYNC_USER
app_servers = settings.MULTIPLE_APP_SERVERS
if not is_file:
path += '/'
log.info('Remote Copy %s to %s', path, target)
Expand Down Expand Up @@ -143,7 +142,7 @@ def copy(cls, path, target, host, is_file=False, **kwargs): # pylint: disable=a

Respects the ``MULTIPLE_APP_SERVERS`` setting when copying.
"""
sync_user = getattr(settings, 'SYNC_USER', getpass.getuser())
sync_user = settings.SYNC_USER
saadmk11 marked this conversation as resolved.
Show resolved Hide resolved
if not is_file:
path += '/'
log.info('Remote Pull %s to %s', path, target)
Expand Down
14 changes: 4 additions & 10 deletions readthedocs/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,12 @@
INVALID_KEYS_COMBINATION = 'invalid-keys-combination'
INVALID_KEY = 'invalid-key'

DOCKER_DEFAULT_IMAGE = getattr(
settings, 'DOCKER_DEFAULT_IMAGE', 'readthedocs/build'
)
DOCKER_DEFAULT_VERSION = getattr(settings, 'DOCKER_DEFAULT_VERSION', '2.0')
DOCKER_DEFAULT_IMAGE = settings.DOCKER_DEFAULT_IMAGE
DOCKER_DEFAULT_VERSION = settings.DOCKER_DEFAULT_VERSION
# These map to corresponding settings in the .org,
# so they haven't been renamed.
DOCKER_IMAGE = getattr(
settings,
'DOCKER_IMAGE',
'{}:{}'.format(DOCKER_DEFAULT_IMAGE, DOCKER_DEFAULT_VERSION),
)
DOCKER_IMAGE_SETTINGS = getattr(settings, 'DOCKER_IMAGE_SETTINGS', {})
DOCKER_IMAGE = settings.DOCKER_IMAGE
DOCKER_IMAGE_SETTINGS = settings.DOCKER_IMAGE_SETTINGS
Copy link
Member

Choose a reason for hiding this comment

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

Why we are re-defining these here instead of using them directly from the settings. object in the code?

Copy link
Member Author

Choose a reason for hiding this comment

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

@humitos I did not change it as it was already like this. And also they are being called in many parts of the config.py file. should I use them directly from the settings in the places of config.py?

Copy link
Member

Choose a reason for hiding this comment

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

I vote to do that in another PR


LATEST_CONFIGURATION_VERSION = 2

Expand Down
25 changes: 9 additions & 16 deletions readthedocs/core/context_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,14 @@
def readthedocs_processor(request):
# pylint: disable=unused-argument
exports = {
'PUBLIC_DOMAIN': getattr(settings, 'PUBLIC_DOMAIN', None),
'PRODUCTION_DOMAIN': getattr(settings, 'PRODUCTION_DOMAIN', None),
'USE_SUBDOMAINS': getattr(settings, 'USE_SUBDOMAINS', None),
'GLOBAL_ANALYTICS_CODE': getattr(settings, 'GLOBAL_ANALYTICS_CODE'),
'DASHBOARD_ANALYTICS_CODE': getattr(
settings,
'DASHBOARD_ANALYTICS_CODE',
),
'SITE_ROOT': getattr(settings, 'SITE_ROOT', '') + '/',
'TEMPLATE_ROOT': getattr(settings, 'TEMPLATE_ROOT', '') + '/',
'DO_NOT_TRACK_ENABLED': getattr(
settings,
'DO_NOT_TRACK_ENABLED',
False,
),
'USE_PROMOS': getattr(settings, 'USE_PROMOS', False),
'PUBLIC_DOMAIN': settings.PUBLIC_DOMAIN,
'PRODUCTION_DOMAIN': settings.PRODUCTION_DOMAIN,
'USE_SUBDOMAINS': settings.USE_SUBDOMAIN,
Copy link
Member

Choose a reason for hiding this comment

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

We should rename the key too (I just checked, and there is not other places using this)

Copy link
Member Author

Choose a reason for hiding this comment

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

sure!

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 Done.:)

'GLOBAL_ANALYTICS_CODE': settings.GLOBAL_ANALYTICS_CODE,
'DASHBOARD_ANALYTICS_CODE': settings.DASHBOARD_ANALYTICS_CODE,
'SITE_ROOT': settings.SITE_ROOT + '/',
'TEMPLATE_ROOT': settings.TEMPLATE_ROOT + '/',
'DO_NOT_TRACK_ENABLED': settings.DO_NOT_TRACK_ENABLED,
'USE_PROMOS': settings.USE_PROMOS,
}
return exports
22 changes: 5 additions & 17 deletions readthedocs/core/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,8 @@
log = logging.getLogger(__name__)

LOG_TEMPLATE = '(Middleware) {msg} [{host}{path}]'
SUBDOMAIN_URLCONF = getattr(
settings,
'SUBDOMAIN_URLCONF',
'readthedocs.core.urls.subdomain',
)
SINGLE_VERSION_URLCONF = getattr(
settings,
'SINGLE_VERSION_URLCONF',
'readthedocs.core.urls.single_version',
)
SUBDOMAIN_URLCONF = settings.SUBDOMAIN_URLCONF
SINGLE_VERSION_URLCONF = settings.SINGLE_VERSION_URLCONF


class SubdomainMiddleware(MiddlewareMixin):
Expand All @@ -39,18 +31,14 @@ def process_request(self, request):
is not set and the request is for a subdomain on ``PRODUCTION_DOMAIN``,
process the request as a request a documentation project.
"""
if not getattr(settings, 'USE_SUBDOMAIN', False):
if not settings.USE_SUBDOMAIN:
return None

full_host = host = request.get_host().lower()
path = request.get_full_path()
log_kwargs = dict(host=host, path=path)
public_domain = getattr(settings, 'PUBLIC_DOMAIN', None)
production_domain = getattr(
settings,
'PRODUCTION_DOMAIN',
'readthedocs.org',
)
public_domain = settings.PUBLIC_DOMAIN
production_domain = settings.PRODUCTION_DOMAIN

if public_domain is None:
public_domain = production_domain
Expand Down
20 changes: 8 additions & 12 deletions readthedocs/core/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ def resolve_domain(self, project, private=None):
if self._use_subdomain():
return self._get_project_subdomain(canonical_project)

return getattr(settings, 'PRODUCTION_DOMAIN')
return settings.PRODUCTION_DOMAIN

def resolve(
self, project, require_https=False, filename='', query_params='',
Expand All @@ -178,10 +178,10 @@ def resolve(
elif self._use_subdomain():
domain = self._get_project_subdomain(canonical_project)
else:
domain = getattr(settings, 'PRODUCTION_DOMAIN')
domain = settings.PRODUCTION_DOMAIN

public_domain = getattr(settings, 'PUBLIC_DOMAIN', None)
use_https = getattr(settings, 'PUBLIC_DOMAIN_USES_HTTPS', False)
public_domain = settings.PUBLIC_DOMAIN
use_https = settings.PUBLIC_DOMAIN_USES_HTTPS

use_https_protocol = any([
# Rely on the ``Domain.https`` field
Expand Down Expand Up @@ -229,7 +229,7 @@ def _get_canonical_project(self, project, projects=None):

def _get_project_subdomain(self, project):
"""Determine canonical project domain as subdomain."""
public_domain = getattr(settings, 'PUBLIC_DOMAIN', None)
public_domain = settings.PUBLIC_DOMAIN
if self._use_subdomain():
project = self._get_canonical_project(project)
subdomain_slug = project.slug.replace('_', '-')
Expand All @@ -241,11 +241,7 @@ def _get_private(self, project, version_slug):
version = project.versions.get(slug=version_slug)
private = version.privacy_level == PRIVATE
except Version.DoesNotExist:
private = getattr(
settings,
'DEFAULT_PRIVACY_LEVEL',
PUBLIC,
) == PRIVATE
private = settings.DEFAULT_PRIVACY_LEVEL == PRIVATE
return private

def _fix_filename(self, project, filename):
Expand All @@ -270,8 +266,8 @@ def _use_custom_domain(self, custom_domain):

def _use_subdomain(self):
"""Make decision about whether to use a subdomain to serve docs."""
use_subdomain = getattr(settings, 'USE_SUBDOMAIN', False)
public_domain = getattr(settings, 'PUBLIC_DOMAIN', None)
use_subdomain = settings.USE_SUBDOMAIN
public_domain = settings.PUBLIC_DOMAIN
return use_subdomain and public_domain is not None


Expand Down
2 changes: 1 addition & 1 deletion readthedocs/core/templatetags/core_tags.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def restructuredtext(value, short=False):
'file_insertion_enabled': False,
}
docutils_settings.update(
getattr(settings, 'RESTRUCTUREDTEXT_FILTER_SETTINGS', {}),
settings.RESTRUCTUREDTEXT_FILTER_SETTINGS,
)
try:
parts = publish_parts(
Expand Down
4 changes: 2 additions & 2 deletions readthedocs/core/urls/single_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@
groups = [single_version_urls]

# Needed to serve media locally
if getattr(settings, 'DEBUG', False):
if settings.DEBUG:
groups.insert(
0,
static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT),
)

# Allow `/docs/<foo>` URL's when not using subdomains or during local dev
if not getattr(settings, 'USE_SUBDOMAIN', False) or settings.DEBUG:
if not settings.USE_SUBDOMAIN or settings.DEBUG:
docs_url = [
url(
(
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/core/urls/subdomain.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
groups = [subdomain_urls]

# Needed to serve media locally
if getattr(settings, 'DEBUG', False):
if settings.DEBUG:
groups.insert(
0,
static(
Expand Down
9 changes: 4 additions & 5 deletions readthedocs/core/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from __future__ import absolute_import

import errno
import getpass
import logging
import os
import re
Expand All @@ -21,7 +20,7 @@

log = logging.getLogger(__name__)

SYNC_USER = getattr(settings, 'SYNC_USER', getpass.getuser())
SYNC_USER = settings.SYNC_USER


def broadcast(type, task, args, kwargs=None, callback=None): # pylint: disable=redefined-builtin
Expand All @@ -37,11 +36,11 @@ def broadcast(type, task, args, kwargs=None, callback=None): # pylint: disable=
raise ValueError('allowed value of `type` are web, app and build.')
if kwargs is None:
kwargs = {}
default_queue = getattr(settings, 'CELERY_DEFAULT_QUEUE', 'celery')

if type in ['web', 'app']:
servers = getattr(settings, 'MULTIPLE_APP_SERVERS', [default_queue])
servers = settings.MULTIPLE_APP_SERVERS
elif type in ['build']:
servers = getattr(settings, 'MULTIPLE_BUILD_SERVERS', [default_queue])
servers = settings.MULTIPLE_BUILD_SERVERS

tasks = []
for server in servers:
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/core/utils/extend.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def get_override_class(proxy_class, default_class=None):
inspect.getmodule(proxy_class).__name__,
proxy_class.__name__,
])
class_path = getattr(settings, 'CLASS_OVERRIDES', {}).get(class_id)
class_path = settings.CLASS_OVERRIDES.get(class_id)
# pylint: disable=protected-access
if class_path is None and proxy_class._override_setting is not None:
class_path = getattr(settings, proxy_class._override_setting, None)
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/core/utils/tasks/public.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
'get_public_task_data',
)

STATUS_UPDATES_ENABLED = not getattr(settings, 'CELERY_ALWAYS_EAGER', False)
STATUS_UPDATES_ENABLED = not settings.CELERY_ALWAYS_EAGER


class PublicTask(Task):
Expand Down
8 changes: 2 additions & 6 deletions readthedocs/core/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,10 @@ class SupportView(TemplateView):

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
support_email = getattr(settings, 'SUPPORT_EMAIL', None)
support_email = settings.SUPPORT_EMAIL
if not support_email:
support_email = 'support@{domain}'.format(
domain=getattr(
settings,
'PRODUCTION_DOMAIN',
'readthedocs.org',
),
domain=settings.PRODUCTION_DOMAIN
)

context['support_email'] = support_email
Expand Down
4 changes: 2 additions & 2 deletions readthedocs/core/views/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ def _serve_file(request, filename, basepath):
:raises: ``Http404`` on ``UnicodeEncodeError``
"""
# Serve the file from the proper location
if settings.DEBUG or getattr(settings, 'PYTHON_MEDIA', False):
if settings.DEBUG or settings.PYTHON_MEDIA:
# Serve from Python
return serve(request, filename, basepath)

Expand Down Expand Up @@ -242,7 +242,7 @@ def _serve_symlink_docs(request, project, privacy_level, filename=''):

files_tried = []

serve_docs = getattr(settings, 'SERVE_DOCS', [constants.PRIVATE])
serve_docs = settings.SERVE_DOCS

if (settings.DEBUG or constants.PUBLIC in serve_docs) and privacy_level != constants.PRIVATE: # yapf: disable # noqa
public_symlink = PublicSymlink(project)
Expand Down
14 changes: 3 additions & 11 deletions readthedocs/doc_builder/backends/mkdocs.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def get_absolute_static_url():
static_url = settings.STATIC_URL

if not static_url.startswith('http'):
domain = getattr(settings, 'PRODUCTION_DOMAIN')
domain = settings.PRODUCTION_DOMAIN
static_url = 'http://{}{}'.format(domain, static_url)

return static_url
Expand Down Expand Up @@ -213,18 +213,10 @@ def generate_rtd_data(self, docs_dir, mkdocs_config):
'builder': 'mkdocs',
'docroot': docs_dir,
'source_suffix': '.md',
'api_host': getattr(
settings,
'PUBLIC_API_URL',
'https://readthedocs.org',
),
'api_host': settings.PUBLIC_API_URL,
'ad_free': not self.project.show_advertising,
'commit': self.version.project.vcs_repo(self.version.slug).commit,
'global_analytics_code': getattr(
settings,
'GLOBAL_ANALYTICS_CODE',
'UA-17997319-1',
),
'global_analytics_code': settings.GLOBAL_ANALYTICS_CODE,
'user_analytics_code': analytics_code,
}
data_json = json.dumps(readthedocs_data, indent=4)
Expand Down
8 changes: 2 additions & 6 deletions readthedocs/doc_builder/backends/sphinx.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def get_config_params(self):
display_gitlab = gitlab_user is not None

# Avoid hitting database and API if using Docker build environment
if getattr(settings, 'DONT_HIT_API', False):
if settings.DONT_HIT_API:
versions = self.project.active_versions()
downloads = self.version.get_downloads(pretty=True)
else:
Expand All @@ -119,11 +119,7 @@ def get_config_params(self):
'version': self.version,
'settings': settings,
'conf_py_path': conf_py_path,
'api_host': getattr(
settings,
'PUBLIC_API_URL',
'https://readthedocs.org',
),
'api_host': settings.PUBLIC_API_URL,
'commit': self.project.vcs_repo(self.version.slug).commit,
'versions': versions,
'downloads': downloads,
Expand Down
Loading