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

ref(py3): pre-commit upgrade + sweeping rerun + __future__ removals #23197

Merged
merged 18 commits into from
Jan 25, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
16 changes: 7 additions & 9 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,22 @@ exclude: >
^src/sentry/data/
)
repos:
- repo: https://github.com/ambv/black
rev: 19.10b0
- repo: local
hooks:
# Configuration for black exists in pyproject.toml.
- id: black
name: black
entry: black
language: python
types: [python]
exclude: (migrations/)
- repo: local
joshuarli marked this conversation as resolved.
Show resolved Hide resolved
hooks:
- id: flake8
name: flake8
entry: flake8
language: python
files: \.py$
types: [python]
log_file: '.artifacts/flake8.pycodestyle.log'
additional_dependencies:
- sentry-flake8==0.4.0
- repo: git://github.com/pre-commit/pre-commit-hooks
rev: v2.5.0
rev: v3.4.0
hooks:
- id: check-case-conflict
- id: check-executables-have-shebangs
Expand Down
3 changes: 1 addition & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ setup-git: ensure-venv setup-git-config
@echo "--> Installing git hooks"
mkdir -p .git/hooks && cd .git/hooks && ln -sf ../../config/hooks/* ./
@python3 -c '' || (echo 'Please run `make setup-pyenv` to install the required Python 3 version.'; exit 1)
@# pre-commit loosely pins virtualenv, which has caused problems in the past.
$(PIP) install "pre-commit==1.18.2" "virtualenv==20.0.32"
$(PIP) install -r requirements-pre-commit.txt
joshuarli marked this conversation as resolved.
Show resolved Hide resolved
@pre-commit install --install-hooks
@echo ""

Expand Down
2 changes: 2 additions & 0 deletions bin/dump-command-help
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#!/usr/bin/env python
from __future__ import absolute_import
joshuarli marked this conversation as resolved.
Show resolved Hide resolved

import os
import six
import click
Expand Down
6 changes: 4 additions & 2 deletions bin/find-good-catalogs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#!/usr/bin/env python

from __future__ import absolute_import

import os
import json
import json # noqa: b317
import click

from babel.messages.pofile import read_po
Expand All @@ -11,7 +13,7 @@ MINIMUM = 80


def is_translated(msg):
if isinstance(msg.string, basestring):
if isinstance(msg.string, bytes):
return bool(msg.string)
for item in msg.string:
if not item:
Expand Down
8 changes: 5 additions & 3 deletions bin/load-mocks
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ from sentry.models import (
)
from sentry.signals import mocks_loaded
from sentry.similarity import features
from sentry.snuba.models import QueryAggregations
from sentry.utils import loremipsum
from sentry.utils.hashlib import md5_text
from sentry.utils.samples import create_sample_event as _create_sample_event
Expand Down Expand Up @@ -164,7 +163,7 @@ def generate_tombstones(project, user):
GroupTombstone.objects.create(
previous_group_id=prev_group_id,
actor_id=user.id,
**{name: getattr(group, name) for name in TOMBSTONE_FIELDS_FROM_GROUP}
**{name: getattr(group, name) for name in TOMBSTONE_FIELDS_FROM_GROUP},
)
prev_group_id += 1

Expand Down Expand Up @@ -260,7 +259,10 @@ def create_sample_time_series(event, release=None):
int(count * 1.1),
)
tsdb.incr(
tsdb.models.project_total_forwarded, project.id, now, int(count * 1.1),
tsdb.models.project_total_forwarded,
project.id,
now,
int(count * 1.1),
)
tsdb.incr_multi(
(
Expand Down
2 changes: 2 additions & 0 deletions bin/merge-catalogs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#!/usr/bin/env python

from __future__ import absolute_import

import os
import click

Expand Down
5 changes: 3 additions & 2 deletions bin/mock-event
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#!/usr/bin/env python
# isort:skip_file
from __future__ import absolute_import

from sentry.runner import configure

configure()
Expand Down Expand Up @@ -28,7 +29,7 @@ def main(project, sample_type):
if not project.first_event:
project.update(first_event=timezone.now())

print("> Created event {}".format(event.event_id))
sys.stdout.write("> Created event {}\n".format(event.event_id))


if __name__ == "__main__":
Expand Down
3 changes: 2 additions & 1 deletion bin/mock-user
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#!/usr/bin/env python
# isort:skip_file
from __future__ import absolute_import

from sentry.runner import configure

configure()
Expand Down
9 changes: 5 additions & 4 deletions config/hooks/post-merge
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ trap "rm -f ${files_changed_upstream}" EXIT

git diff-tree -r --name-only --no-commit-id ORIG_HEAD HEAD > "$files_changed_upstream"

grep -E --quiet 'requirements.*\.txt' "$files_changed_upstream" && py="install-py-dev "
grep -E --quiet 'yarn.lock' "$files_changed_upstream" && js="install-js-dev "
grep -E --quiet 'migrations' "$files_changed_upstream" && migrations="apply-migrations "
grep -E --quiet 'requirements-pre-commit\.txt' "$files_changed_upstream" && pc="setup-git "
joshuarli marked this conversation as resolved.
Show resolved Hide resolved
grep -E --quiet 'requirements-(base|dev)\.txt' "$files_changed_upstream" && py="install-py-dev "
grep -E --quiet 'yarn\.lock' "$files_changed_upstream" && js="install-js-dev "
grep -E --quiet 'migrations' "$files_changed_upstream" && migrations="apply-migrations "

[[ "$py" || "$js" || "$migrations" ]] && needs_update=1
[[ "$pc" || "$py" || "$js" || "$migrations" ]] && needs_update=1
update_command="make ${py}${js}${migrations}"

[[ "$needs_update" ]] && cat <<EOF
Expand Down
2 changes: 1 addition & 1 deletion config/hooks/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import sys

from glob import glob

text_type = type(u"")
text_type = type("")

# git usurbs your bin path for hooks and will always run system python
if "VIRTUAL_ENV" in os.environ:
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"

[tool.black]
line-length=100
target-version=['py27']
target-version=['py36']
include='\.py$'
exclude='''
(
Expand Down
3 changes: 3 additions & 0 deletions requirements-pre-commit.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pre-commit==2.9.3
black==20.8b1
sentry-flake8==0.4.0
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def run(self):


def get_requirements(env):
with open(u"requirements-{}.txt".format(env)) as fp:
with open("requirements-{}.txt".format(env)) as fp:
return [x.strip() for x in fp.read().split("\n") if not x.startswith("#")]


Expand Down
2 changes: 1 addition & 1 deletion src/sentry/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class OptionAdmin(admin.ModelAdmin):
search_fields = ("key",)

def value_repr(self, instance):
return u'<pre style="display:inline-block;white-space:pre-wrap;">{}</pre>'.format(
return '<pre style="display:inline-block;white-space:pre-wrap;">{}</pre>'.format(
escape(saferepr(instance.value))
)

Expand Down
8 changes: 4 additions & 4 deletions src/sentry/analytics/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ def extract(self, value):
for attr in self.attributes:
nv = items.pop(attr.name, None)
if attr.required and nv is None:
raise ValueError(u"{} is required (cannot be None)".format(attr.name))
raise ValueError("{} is required (cannot be None)".format(attr.name))

data[attr.name] = attr.extract(nv)

if items:
raise ValueError(
u"Unknown attributes: {}".format(", ".join(map(six.text_type, six.iterkeys(items))))
"Unknown attributes: {}".format(", ".join(map(six.text_type, six.iterkeys(items))))
)

return data
Expand All @@ -89,11 +89,11 @@ def __init__(self, type=None, datetime=None, **items):
for attr in self.attributes:
nv = items.pop(attr.name, None)
if attr.required and nv is None:
raise ValueError(u"{} is required (cannot be None)".format(attr.name))
raise ValueError("{} is required (cannot be None)".format(attr.name))
data[attr.name] = attr.extract(nv)

if items:
raise ValueError(u"Unknown attributes: {}".format(", ".join(six.iterkeys(items))))
raise ValueError("Unknown attributes: {}".format(", ".join(six.iterkeys(items))))

self.data = data

Expand Down
19 changes: 11 additions & 8 deletions src/sentry/api/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
ONE_HOUR = ONE_MINUTE * 60
ONE_DAY = ONE_HOUR * 24

LINK_HEADER = u'<{uri}&cursor={cursor}>; rel="{name}"; results="{has_results}"; cursor="{cursor}"'
LINK_HEADER = '<{uri}&cursor={cursor}>; rel="{name}"; results="{has_results}"; cursor="{cursor}"'

DEFAULT_AUTHENTICATION = (TokenAuthentication, ApiKeyAuthentication, SessionAuthentication)

Expand Down Expand Up @@ -98,14 +98,14 @@ class Endpoint(APIView):
permission_classes = (NoPermission,)

def build_cursor_link(self, request, name, cursor):
querystring = u"&".join(
u"{0}={1}".format(urlquote(k), urlquote(v))
querystring = "&".join(
"{0}={1}".format(urlquote(k), urlquote(v))
for k, v in six.iteritems(request.GET)
if k != "cursor"
)
base_url = absolute_uri(urlquote(request.path))
if querystring:
base_url = u"{0}?{1}".format(base_url, querystring)
base_url = "{0}?{1}".format(base_url, querystring)
else:
base_url = base_url + "?"

Expand Down Expand Up @@ -249,7 +249,8 @@ def dispatch(self, request, *args, **kwargs):

if duration < (settings.SENTRY_API_RESPONSE_DELAY / 1000.0):
with sentry_sdk.start_span(
op="base.dispatch.sleep", description=type(self).__name__,
op="base.dispatch.sleep",
description=type(self).__name__,
) as span:
span.set_data("SENTRY_API_RESPONSE_DELAY", settings.SENTRY_API_RESPONSE_DELAY)
time.sleep(settings.SENTRY_API_RESPONSE_DELAY / 1000.0 - duration)
Expand Down Expand Up @@ -297,7 +298,7 @@ def paginate(
paginator_cls=Paginator,
default_per_page=100,
max_per_page=100,
**paginator_kwargs
**paginator_kwargs,
):
assert (paginator and not paginator_kwargs) or (paginator_cls and paginator_kwargs)

Expand All @@ -315,7 +316,8 @@ def paginate(

try:
with sentry_sdk.start_span(
op="base.paginate.get_result", description=type(self).__name__,
op="base.paginate.get_result",
description=type(self).__name__,
) as span:
span.set_data("Limit", per_page)
cursor_result = paginator.get_result(limit=per_page, cursor=input_cursor)
Expand All @@ -325,7 +327,8 @@ def paginate(
# map results based on callback
if on_results:
with sentry_sdk.start_span(
op="base.paginate.on_results", description=type(self).__name__,
op="base.paginate.on_results",
description=type(self).__name__,
):
results = on_results(cursor_result.results)
else:
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/api/bases/avatar.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def get_serializer_context(self, obj, **kwargs):
return {"type": self.model, "kwargs": {self.object_type: obj}}

def get_avatar_filename(self, obj):
return u"{}.png".format(obj.id)
return "{}.png".format(obj.id)

def put(self, request, **kwargs):
obj = kwargs[self.object_type]
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/api/bases/organization.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ def get_filter_params(
with configure_scope() as scope:
scope.set_tag("query.period", (end - start).total_seconds())
except InvalidParams as e:
raise ParseError(detail=u"Invalid date range: {}".format(e))
raise ParseError(detail="Invalid date range: {}".format(e))

try:
projects = self.get_projects(request, organization, project_ids)
Expand Down
6 changes: 3 additions & 3 deletions src/sentry/api/bases/organization_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,16 +149,16 @@ class OrganizationEventsV2EndpointBase(OrganizationEventsEndpointBase):
def build_cursor_link(self, request, name, cursor):
# The base API function only uses the last query parameter, but this endpoint
# needs all the parameters, particularly for the "field" query param.
querystring = u"&".join(
u"{0}={1}".format(urlquote(query[0]), urlquote(value))
querystring = "&".join(
"{0}={1}".format(urlquote(query[0]), urlquote(value))
for query in request.GET.lists()
if query[0] != "cursor"
for value in query[1]
)

base_url = absolute_uri(urlquote(request.path))
if querystring:
base_url = u"{0}?{1}".format(base_url, querystring)
base_url = "{0}?{1}".format(base_url, querystring)
else:
base_url = base_url + "?"

Expand Down
3 changes: 2 additions & 1 deletion src/sentry/api/bases/organization_integrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ def get_organization_integration(organization, integration_id):
"""
try:
return OrganizationIntegration.objects.get(
integration_id=integration_id, organization=organization,
integration_id=integration_id,
organization=organization,
)
except OrganizationIntegration.DoesNotExist:
raise Http404
Expand Down
6 changes: 3 additions & 3 deletions src/sentry/api/bases/sentryapps.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class SentryAppsBaseEndpoint(IntegrationPlatformEndpoint):
def _get_organization_slug(self, request):
organization_slug = request.json_body.get("organization")
if not organization_slug or not isinstance(organization_slug, string_types):
error_message = u"""
error_message = """
Please provide a valid value for the 'organization' field.
"""
raise ValidationError({"organization": to_single_line_str(error_message)})
Expand All @@ -116,7 +116,7 @@ def _get_organization_for_superuser(self, organization_slug):
try:
return Organization.objects.get(slug=organization_slug)
except Organization.DoesNotExist:
error_message = u"""
error_message = """
Organization '{}' does not exist.
""".format(
organization_slug
Expand All @@ -127,7 +127,7 @@ def _get_organization_for_user(self, user, organization_slug):
try:
return user.get_orgs().get(slug=organization_slug)
except Organization.DoesNotExist:
error_message = u"""
error_message = """
User does not belong to the '{}' organization.
""".format(
organization_slug
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/api/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ def __init__(self, status_code, body):
self.body = body

def __str__(self):
return u"status={} body={}".format(self.status_code, self.body)
return "status={} body={}".format(self.status_code, self.body)

def __repr__(self):
return u"<ApiError: {}>".format(self)
return "<ApiError: {}>".format(self)


class ApiClient(object):
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/api/endpoints/auth_login.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def post(self, request, organization=None, *args, **kwargs):

# Rate limit logins
is_limited = ratelimiter.is_limited(
u"auth:login:username:{}".format(
"auth:login:username:{}".format(
md5_text(login_form.clean_username(request.data.get("username"))).hexdigest()
),
limit=10,
Expand Down
3 changes: 1 addition & 2 deletions src/sentry/api/endpoints/authenticator_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ class AuthenticatorIndexEndpoint(Endpoint):
permission_classes = (IsAuthenticated,)

def get(self, request):
"""Returns u2f interface for a user, otherwise an empty array
"""
"""Returns u2f interface for a user, otherwise an empty array"""

# Currently just expose u2f challenge, not sure if it's necessary to list all
# authenticator interfaces that are enabled
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/api/endpoints/event_apple_crash_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def get(self, request, project, event_id):
response = HttpResponse(apple_crash_report_string, content_type="text/plain")

if request.GET.get("download") is not None:
filename = u"{}{}.crash".format(event.event_id, symbolicated and "-symbolicated" or "")
filename = "{}{}.crash".format(event.event_id, symbolicated and "-symbolicated" or "")
response = StreamingHttpResponse(apple_crash_report_string, content_type="text/plain")
response["Content-Length"] = len(apple_crash_report_string)
response["Content-Disposition"] = 'attachment; filename="%s"' % filename
Expand Down