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

Requirements: update structlog and django-structlog #11166

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
114 changes: 60 additions & 54 deletions readthedocs/api/v2/views/integrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class WebhookMixin:
renderer_classes = (JSONRenderer,)
integration = None
integration_type = None
invalid_payload_msg = 'Payload not valid'
invalid_payload_msg = "Payload not valid"
missing_secret_deprecated_msg = dedent(
"""
This webhook doesn't have a secret configured.
Expand All @@ -84,7 +84,7 @@ def post(self, request, project_slug):
"""Set up webhook post view with request and project objects."""
self.request = request

log.bind(
structlog.contextvars.bind_contextvars(
project_slug=project_slug,
integration_type=self.integration_type,
)
Expand All @@ -101,7 +101,7 @@ def post(self, request, project_slug):
try:
self.project = self.get_project(slug=project_slug)
if not Project.objects.is_active(self.project):
resp = {'detail': 'This project is currently disabled'}
resp = {"detail": "This project is currently disabled"}
return Response(resp, status=status.HTTP_406_NOT_ACCEPTABLE)
except Project.DoesNotExist as exc:
raise NotFound("Project not found") from exc
Expand All @@ -115,15 +115,15 @@ def post(self, request, project_slug):
)

if not self.is_payload_valid():
log.warning('Invalid payload for project and integration.')
log.warning("Invalid payload for project and integration.")
return Response(
{'detail': self.invalid_payload_msg},
{"detail": self.invalid_payload_msg},
status=HTTP_400_BAD_REQUEST,
)
resp = self.handle_webhook()
if resp is None:
log.info('Unhandled webhook event')
resp = {'detail': 'Unhandled webhook event'}
log.info("Unhandled webhook event")
resp = {"detail": "Unhandled webhook event"}

# The response can be a DRF Response with with the status code already set.
# In that case, we just return it as is.
Expand All @@ -143,7 +143,7 @@ def get_project(self, **kwargs):
def finalize_response(self, req, *args, **kwargs):
"""If the project was set on POST, store an HTTP exchange."""
resp = super().finalize_response(req, *args, **kwargs)
if hasattr(self, 'project') and self.project:
if hasattr(self, "project") and self.project:
HttpExchange.objects.from_exchange(
req,
resp,
Expand Down Expand Up @@ -222,14 +222,14 @@ def get_response_push(self, project, branches):
to_build, not_building = build_branches(project, branches)
if not_building:
log.info(
'Skipping project branches.',
"Skipping project branches.",
branches=branches,
)
triggered = bool(to_build)
return {
'build_triggered': triggered,
'project': project.slug,
'versions': list(to_build),
"build_triggered": triggered,
"project": project.slug,
"versions": list(to_build),
}

def sync_versions_response(self, project, sync=True):
Expand All @@ -242,10 +242,10 @@ def sync_versions_response(self, project, sync=True):
if sync:
version = trigger_sync_versions(project)
return {
'build_triggered': False,
'project': project.slug,
'versions': [version] if version else [],
'versions_synced': version is not None,
"build_triggered": False,
"project": project.slug,
"versions": [version] if version else [],
"versions_synced": version is not None,
}

def get_external_version_response(self, project):
Expand Down Expand Up @@ -372,12 +372,12 @@ class GitHubWebhookView(WebhookMixin, APIView):
"""

integration_type = Integration.GITHUB_WEBHOOK
invalid_payload_msg = 'Payload not valid, invalid or missing signature'
invalid_payload_msg = "Payload not valid, invalid or missing signature"

def get_data(self):
if self.request.content_type == 'application/x-www-form-urlencoded':
if self.request.content_type == "application/x-www-form-urlencoded":
try:
return json.loads(self.request.data['payload'])
return json.loads(self.request.data["payload"])
except (ValueError, KeyError):
pass
return super().get_data()
Expand Down Expand Up @@ -446,11 +446,11 @@ def handle_webhook(self):

"""
# Get event and trigger other webhook events
action = self.data.get('action', None)
created = self.data.get('created', False)
deleted = self.data.get('deleted', False)
action = self.data.get("action", None)
created = self.data.get("created", False)
deleted = self.data.get("deleted", False)
event = self.request.headers.get(GITHUB_EVENT_HEADER, GITHUB_PUSH)
log.bind(webhook_event=event)
structlog.contextvars.bind_contextvars(webhook_event=event)
webhook_github.send(
Project,
project=self.project,
Expand All @@ -469,7 +469,7 @@ def handle_webhook(self):

# Sync versions when a branch/tag was created/deleted
if event in (GITHUB_CREATE, GITHUB_DELETE):
log.debug('Triggered sync_versions.')
log.debug("Triggered sync_versions.")
return self.sync_versions_response(self.project)

integration = self.get_integration()
Expand All @@ -489,22 +489,30 @@ def handle_webhook(self):
return self.get_closed_external_version_response(self.project)

# Sync versions when push event is created/deleted action
if all([
if all(
[
event == GITHUB_PUSH,
(created or deleted),
]):
events = integration.provider_data.get('events', []) if integration.provider_data else [] # noqa
if any([
]
):
events = (
integration.provider_data.get("events", [])
if integration.provider_data
else []
) # noqa
if any(
[
GITHUB_CREATE in events,
GITHUB_DELETE in events,
]):
]
):
# GitHub will send PUSH **and** CREATE/DELETE events on a creation/deletion in newer
# webhooks. If we receive a PUSH event we need to check if the webhook doesn't
# already have the CREATE/DELETE events. So we don't trigger the sync twice.
return self.sync_versions_response(self.project, sync=False)

log.debug(
'Triggered sync_versions.',
"Triggered sync_versions.",
integration_events=events,
)
return self.sync_versions_response(self.project)
Expand All @@ -521,8 +529,8 @@ def handle_webhook(self):

def _normalize_ref(self, ref):
"""Remove `ref/(heads|tags)/` from the reference to match a Version on the db."""
pattern = re.compile(r'^refs/(heads|tags)/')
return pattern.sub('', ref)
pattern = re.compile(r"^refs/(heads|tags)/")
return pattern.sub("", ref)


class GitLabWebhookView(WebhookMixin, APIView):
Expand Down Expand Up @@ -565,7 +573,7 @@ class GitLabWebhookView(WebhookMixin, APIView):
"""

integration_type = Integration.GITLAB_WEBHOOK
invalid_payload_msg = 'Payload not valid, invalid or missing token'
invalid_payload_msg = "Payload not valid, invalid or missing token"

def is_payload_valid(self):
"""
Expand Down Expand Up @@ -602,9 +610,9 @@ def handle_webhook(self):
instead, it sets the before/after field to
0000000000000000000000000000000000000000 ('0' * 40)
"""
event = self.request.data.get('object_kind', GITLAB_PUSH)
action = self.data.get('object_attributes', {}).get('action', None)
log.bind(webhook_event=event)
event = self.request.data.get("object_kind", GITLAB_PUSH)
action = self.data.get("object_attributes", {}).get("action", None)
structlog.contextvars.bind_contextvars(webhook_event=event)
webhook_gitlab.send(
Project,
project=self.project,
Expand All @@ -621,12 +629,12 @@ def handle_webhook(self):
# Handle push events and trigger builds
if event in (GITLAB_PUSH, GITLAB_TAG_PUSH):
data = self.request.data
before = data.get('before')
after = data.get('after')
before = data.get("before")
after = data.get("after")
# Tag/branch created/deleted
if GITLAB_NULL_HASH in (before, after):
log.debug(
'Triggered sync_versions.',
"Triggered sync_versions.",
before=before,
after=after,
)
Expand All @@ -653,8 +661,8 @@ def handle_webhook(self):
return None

def _normalize_ref(self, ref):
pattern = re.compile(r'^refs/(heads|tags)/')
return pattern.sub('', ref)
pattern = re.compile(r"^refs/(heads|tags)/")
return pattern.sub("", ref)


class BitbucketWebhookView(WebhookMixin, APIView):
Expand Down Expand Up @@ -700,7 +708,7 @@ def handle_webhook(self):
attribute (null if it is a creation).
"""
event = self.request.headers.get(BITBUCKET_EVENT_HEADER, BITBUCKET_PUSH)
log.bind(webhook_event=event)
structlog.contextvars.bind_contextvars(webhook_event=event)
webhook_bitbucket.send(
Project,
project=self.project,
Expand All @@ -715,14 +723,14 @@ def handle_webhook(self):
if event == BITBUCKET_PUSH:
try:
data = self.request.data
changes = data['push']['changes']
changes = data["push"]["changes"]
branches = []
for change in changes:
old = change['old']
new = change['new']
old = change["old"]
new = change["new"]
# Normal push to master
if old is not None and new is not None:
branches.append(new['name'])
branches.append(new["name"])
# BitBuck returns an array of changes rather than
# one webhook per change. If we have at least one normal push
# we don't trigger the sync versions, because that
Expand Down Expand Up @@ -770,7 +778,7 @@ class IsAuthenticatedOrHasToken(permissions.IsAuthenticated):

def has_permission(self, request, view):
has_perm = super().has_permission(request, view)
return has_perm or 'token' in request.data
return has_perm or "token" in request.data


class APIWebhookView(WebhookMixin, APIView):
Expand Down Expand Up @@ -799,15 +807,13 @@ def get_project(self, **kwargs):
# If the user is not an admin of the project, fall back to token auth
if self.request.user.is_authenticated:
try:
return (
Project.objects.for_admin_user(
self.request.user,
).get(**kwargs)
)
return Project.objects.for_admin_user(
self.request.user,
).get(**kwargs)
except Project.DoesNotExist:
pass
# Recheck project and integration relationship during token auth check
token = self.request.data.get('token')
token = self.request.data.get("token")
if token:
integration = self.get_integration()
obj = Project.objects.get(**kwargs)
Expand All @@ -821,7 +827,7 @@ def get_project(self, **kwargs):
def handle_webhook(self):
try:
branches = self.request.data.get(
'branches',
"branches",
[self.project.get_default_branch()],
)
default_branch = self.request.data.get("default_branch", None)
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/builds/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ def send_build_status(build_pk, commit, status):

provider_name = build.project.git_provider_name

log.bind(
structlog.contextvars.bind_contextvars(
build_id=build.pk,
project_slug=build.project.slug,
commit=commit,
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/core/logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ def __call__(self, logger, method_name, event_dict):


shared_processors = [
structlog.contextvars.merge_contextvars,
structlog.stdlib.add_logger_name,
structlog.stdlib.add_log_level,
structlog.stdlib.PositionalArgumentsFormatter(),
Expand All @@ -219,7 +220,6 @@ def __call__(self, logger, method_name, event_dict):
structlog.stdlib.ProcessorFormatter.wrap_for_formatter,
]
),
context_class=structlog.threadlocal.wrap_dict(dict),
logger_factory=structlog.stdlib.LoggerFactory(),
wrapper_class=structlog.stdlib.BoundLogger,
cache_logger_on_first_use=True,
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/core/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def process_email_confirmed(request, email_address, **kwargs):
profile = UserProfile.objects.filter(user=user).first()
if profile and profile.mailing_list:
# TODO: Unsubscribe users if they unset `mailing_list`.
log.bind(
structlog.contextvars.bind_contextvars(
email=email_address.email,
username=user.username,
)
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/core/unresolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ def unresolve_domain_from_request(self, request):
:returns: A UnresolvedDomain object.
"""
host = self.get_domain_from_host(request.get_host())
log.bind(host=host)
structlog.contextvars.bind_contextvars(host=host)

# Explicit Project slug being passed in.
header_project_slug = request.headers.get("X-RTD-Slug", "").lower()
Expand Down
8 changes: 4 additions & 4 deletions readthedocs/core/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def prepare_build(
from readthedocs.projects.tasks.builds import update_docs_task
from readthedocs.projects.tasks.utils import send_external_build_status

log.bind(project_slug=project.slug)
structlog.contextvars.bind_contextvars(project_slug=project.slug)

if not Project.objects.is_active(project):
log.warning(
Expand All @@ -72,7 +72,7 @@ def prepare_build(
commit=commit,
)

log.bind(
structlog.contextvars.bind_contextvars(
build_id=build.id,
version_slug=version.slug,
)
Expand Down Expand Up @@ -101,7 +101,7 @@ def prepare_build(
options["time_limit"] = int(time_limit * 1.2)

if commit:
log.bind(commit=commit)
structlog.contextvars.bind_contextvars(commit=commit)

# Send pending Build Status using Git Status API for External Builds.
send_external_build_status(
Expand Down Expand Up @@ -194,7 +194,7 @@ def trigger_build(project, version=None, commit=None):
:returns: Celery AsyncResult promise and Build instance
:rtype: tuple
"""
log.bind(
structlog.contextvars.bind_contextvars(
project_slug=project.slug,
version_slug=version.slug if version else None,
commit=commit,
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/core/utils/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def safe_open(

path = Path(path).absolute()

log.bind(
structlog.contextvars.bind_contextvars(
path_resolved=str(path.absolute().resolve()),
)

Expand Down
Loading