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

dev: Add Ruff to API, enable F541 as first rule #555

Merged
merged 3 commits into from
May 10, 2024
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
13 changes: 6 additions & 7 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,18 @@ lint:
make lint.run

lint.install:
python -m pip install --upgrade pip
echo "Installing..."
pip install -Iv black==22.3.0 isort
pip install -Iv ruff

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this file the only non auto generated change in the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and the ruff.toml which has some slight tweaks from the default here: https://docs.astral.sh/ruff/configuration/

Copy link
Contributor

Choose a reason for hiding this comment

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

Are the rules same as the the worker repo?

Copy link
Contributor

Choose a reason for hiding this comment

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

worker is more or less using the default except for the extra linting of F401. It's good that you added the ruff.toml file!

lint.run:
black .
isort --profile black .
ruff check
ruff format

lint.check:
echo "Linting..."
black --check .
echo "Sorting..."
isort --profile black --check .
ruff check
echo "Formatting..."
ruff format --check

build.requirements:
# if docker pull succeeds, we have already build this version of
Expand Down
10 changes: 4 additions & 6 deletions api/internal/feature/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,7 @@ def post(self, request):
# fetch flags not in cache
missed_feature_flags = FeatureFlag.objects.filter(
name__in=cache_misses
).prefetch_related(
"variants"
) # include the feature flag variants aswell
).prefetch_related("variants") # include the feature flag variants aswell

# evaluate the remaining flags
for feature_flag in missed_feature_flags:
Expand All @@ -91,9 +89,9 @@ def post(self, request):
flag_evaluations[feature_flag.name] = Feature(
feature_flag.name, feature_flag, list(feature_flag.variants.all())
).check_value_no_fetch(identifier=identifier)
flags_to_add_to_cache[
get_flag_cache_redis_key(feature_flag.name)
] = feature_flag
flags_to_add_to_cache[get_flag_cache_redis_key(feature_flag.name)] = (
feature_flag
)

# add the new flags to cache
if len(flags_to_add_to_cache) >= 1:
Expand Down
10 changes: 5 additions & 5 deletions api/internal/owner/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def validate_value(self, value):
if value not in plan_values:
if value in SENTRY_PAID_USER_PLAN_REPRESENTATIONS:
log.warning(
f"Non-Sentry user attempted to transition to Sentry plan",
"Non-Sentry user attempted to transition to Sentry plan",
extra=dict(owner_id=current_owner.pk, plan=value),
)
raise serializers.ValidationError(
Expand All @@ -149,27 +149,27 @@ def validate(self, plan):
if plan["value"] in PAID_PLANS:
if "quantity" not in plan:
raise serializers.ValidationError(
f"Field 'quantity' required for updating to paid plans"
"Field 'quantity' required for updating to paid plans"
)
if plan["quantity"] <= 1:
raise serializers.ValidationError(
f"Quantity for paid plan must be greater than 1"
"Quantity for paid plan must be greater than 1"
)

plan_service = PlanService(current_org=owner)
is_org_trialing = plan_service.is_org_trialing

if plan["quantity"] < owner.activated_user_count and not is_org_trialing:
raise serializers.ValidationError(
f"Quantity cannot be lower than currently activated user count"
"Quantity cannot be lower than currently activated user count"
)
if (
plan["quantity"] == owner.plan_user_count
and plan["value"] == owner.plan
and not is_org_trialing
):
raise serializers.ValidationError(
f"Quantity or plan for paid plan must be different from the existing one"
"Quantity or plan for paid plan must be different from the existing one"
)
if (
plan["value"] in TEAM_PLAN_REPRESENTATIONS
Expand Down
2 changes: 1 addition & 1 deletion api/internal/tests/test_charts.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def generate_random_totals(
"m": misses,
"c": coverage,
"C": complexity,
"N": complexity_total
"N": complexity_total,
# Not currenly used: diff, files, sessions, branches, methods
}
return totals
Expand Down
4 changes: 2 additions & 2 deletions api/shared/compare/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ def retrieve(self, request, *args, **kwargs):
elif comparison.pseudo_diff_adjusts_tracked_lines:
return Response(
data={
"detail": f"Changes found in between %.7s...%.7s (pseudo...base) "
f"which prevent comparing this pull request."
"detail": "Changes found in between %.7s...%.7s (pseudo...base) "
"which prevent comparing this pull request."
% (comparison.pull.compared_to, comparison.pull.base)
},
status=400,
Expand Down
1 change: 0 additions & 1 deletion api/shared/compare/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,6 @@ def get_misses_count(self, impacted_file: ImpactedFile) -> serializers.IntegerFi


class ImpactedFilesComparisonSerializer(ComparisonSerializer):

files = serializers.SerializerMethodField()
state = serializers.SerializerMethodField()

Expand Down
14 changes: 8 additions & 6 deletions api/shared/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
from asgiref.sync import async_to_sync
from django.conf import settings
from django.http import Http404
from rest_framework.permissions import SAFE_METHODS # ['GET', 'HEAD', 'OPTIONS']
from rest_framework.permissions import BasePermission
from rest_framework.permissions import (
SAFE_METHODS, # ['GET', 'HEAD', 'OPTIONS']
BasePermission,
)

import services.self_hosted as self_hosted
from api.shared.mixins import InternalPermissionsMixin, SuperPermissionsMixin
Expand Down Expand Up @@ -74,10 +76,10 @@ class RepositoryArtifactPermissions(BasePermission):

permissions_service = RepositoryPermissionsService()
message = (
f"Permission denied: some possible reasons for this are (1) the "
f"user doesn't have permission to view the specific resource, "
f"(2) the organization has a per-user plan or (3) the user is "
f"trying to view a private repo but is not activated."
"Permission denied: some possible reasons for this are (1) the "
"user doesn't have permission to view the specific resource, "
"(2) the organization has a per-user plan or (3) the user is "
"trying to view a private repo but is not activated."
)

def has_permission(self, request, view):
Expand Down
1 change: 0 additions & 1 deletion billing/migrations/0001_initial.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@


class Migration(migrations.Migration):

initial = True

dependencies = []
Expand Down
2 changes: 1 addition & 1 deletion billing/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def _send_event(self, payload):
)
},
data=payload,
format="json"
format="json",
)

def test_invoice_payment_succeeded_sets_owner_delinquent_false(self):
Expand Down
11 changes: 5 additions & 6 deletions billing/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
if updated >= 1:
log.info(f"Successfully updated info for {updated} customer(s)")
else:
log.warning(f"Could not find customer")
log.warning("Could not find customer")

Check warning on line 30 in billing/views.py

View check run for this annotation

Codecov - QA / codecov/patch

billing/views.py#L30

Added line #L30 was not covered by tests

Check warning on line 30 in billing/views.py

View check run for this annotation

Codecov Notifications / codecov/patch

billing/views.py#L30

Added line #L30 was not covered by tests

Check warning on line 30 in billing/views.py

View check run for this annotation

Codecov Public QA / codecov/patch

billing/views.py#L30

Added line #L30 was not covered by tests

Check warning on line 30 in billing/views.py

View check run for this annotation

Codecov / codecov/patch

billing/views.py#L30

Added line #L30 was not covered by tests

def invoice_payment_succeeded(self, invoice):
log.info(
Expand Down Expand Up @@ -113,7 +113,6 @@
)

def subscription_schedule_released(self, schedule):

subscription = stripe.Subscription.retrieve(schedule["released_subscription"])
owner = Owner.objects.get(ownerid=subscription.metadata.obo_organization)
requesting_user_id = subscription.metadata.obo
Expand Down Expand Up @@ -201,8 +200,8 @@
if not subscription_schedule_id:
if subscription.status == "incomplete_expired":
log.info(
f"Subscription updated with status change "
f"to 'incomplete_expired' -- cancelling to free",
"Subscription updated with status change "
"to 'incomplete_expired' -- cancelling to free",
extra=dict(stripe_subscription_id=subscription.id),
)
plan_service.set_default_plan_data()
Expand Down Expand Up @@ -286,13 +285,13 @@
return Response("Invalid signature", status=status.HTTP_400_BAD_REQUEST)
if self.event.type not in StripeWebhookEvents.subscribed_events:
log.warning(
f"Unsupported Stripe webhook event received, exiting",
"Unsupported Stripe webhook event received, exiting",
extra=dict(stripe_webhook_event=self.event.type),
)
return Response("Unsupported event type", status=204)

log.info(
f"Stripe webhook event received",
"Stripe webhook event received",
extra=dict(stripe_webhook_event=self.event.type),
)

Expand Down
6 changes: 3 additions & 3 deletions codecov/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ def save_model(self, request, new_obj, form, change) -> None:
for changed_field in form.changed_data:
prev_value = getattr(old_obj, changed_field)
new_value = getattr(new_obj, changed_field)
new_obj.changed_fields[
changed_field
] = f"prev value: {prev_value}, new value: {new_value}"
new_obj.changed_fields[changed_field] = (
f"prev value: {prev_value}, new value: {new_value}"
)

return super().save_model(request, new_obj, form, change)

Expand Down
2 changes: 1 addition & 1 deletion codecov_auth/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ def refresh(self, obj: OrganizationLevelToken):
# 0 in this case refers to the 0th index of the inline
# But there can only ever be 1 token per org, so it's fine to use that.
return format_html(
f'<input type="checkbox" name="organization_tokens-0-REFRESH" id="id_organization_tokens-0-REFRESH">'
'<input type="checkbox" name="organization_tokens-0-REFRESH" id="id_organization_tokens-0-REFRESH">'
)

def has_change_permission(self, request, obj=None):
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0001_initial.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@


class Migration(migrations.Migration):

initial = True

dependencies = []
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0002_auto_20210817_1346.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@


class Migration(migrations.Migration):

dependencies = [
("core", "0004_pull_user_provided_base_sha"),
("codecov_auth", "0001_initial"),
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0003_auto_20210924_1003.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@


class Migration(migrations.Migration):

dependencies = [("codecov_auth", "0002_auto_20210817_1346")]

operations = [
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0004_auto_20210930_1429.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@


class Migration(migrations.Migration):

dependencies = [("codecov_auth", "0003_auto_20210924_1003")]

operations = [
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0006_auto_20211123_1535.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@


class Migration(migrations.Migration):

dependencies = [("codecov_auth", "0005_auto_20211029_1709")]

operations = [
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0007_auto_20211129_1228.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@


class Migration(migrations.Migration):

dependencies = [("codecov_auth", "0006_auto_20211123_1535")]

operations = [
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0008_auto_20220119_1811.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@


class Migration(migrations.Migration):

dependencies = [("codecov_auth", "0007_auto_20211129_1228")]

operations = [
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0009_auto_20220511_1313.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@


class Migration(migrations.Migration):

dependencies = [
("codecov_auth", "0008_auto_20220119_1811"),
]
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0010_owner_is_superuser.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@


class Migration(migrations.Migration):

dependencies = [
("codecov_auth", "0009_auto_20220511_1313"),
]
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0012_auto_20220531_1452.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@


class Migration(migrations.Migration):

dependencies = [
("codecov_auth", "0011_new_enterprise_plans"),
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@


class Migration(migrations.Migration):

dependencies = [
("codecov_auth", "0012_auto_20220531_1452"),
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@


class Migration(migrations.Migration):

dependencies = [
("codecov_auth", "0013_alter_owner_organizations"),
]
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0015_organizationleveltoken.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@


class Migration(migrations.Migration):

dependencies = [
("codecov_auth", "0014_alter_repositorytoken_token_type"),
]
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0016_alter_owner_admins.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@


class Migration(migrations.Migration):

dependencies = [
("codecov_auth", "0015_organizationleveltoken"),
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@


class Migration(migrations.Migration):

dependencies = [
("codecov_auth", "0016_alter_owner_admins"),
]
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0018_usertoken.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@


class Migration(migrations.Migration):

dependencies = [
("codecov_auth", "0017_alter_organizationleveltoken_token_type"),
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@


class Migration(migrations.Migration):

dependencies = [
("codecov_auth", "0018_usertoken"),
]
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0021_owner_max_upload_limit.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@


class Migration(migrations.Migration):

dependencies = [
("codecov_auth", "0020_ownerprofile_default_org"),
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@


class Migration(migrations.Migration):

dependencies = [
("codecov_auth", "0021_owner_max_upload_limit"),
]
Expand Down
1 change: 0 additions & 1 deletion codecov_auth/migrations/0023_auto_20230214_1129.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@


class Migration(migrations.Migration):

dependencies = [
("codecov_auth", "0022_alter_owner_max_upload_limit"),
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@


class Migration(migrations.Migration):

dependencies = [
("codecov_auth", "0023_auto_20230214_1129"),
]
Expand Down
Loading
Loading