Skip to content

Commit

Permalink
Make changes based on migration to ruff
Browse files Browse the repository at this point in the history
- remove pylint disable lines
- make some changes suggested by ruff
- ignore other changes
  • Loading branch information
grahamalama committed Jul 30, 2024
1 parent 6cd0427 commit 402e045
Show file tree
Hide file tree
Showing 15 changed files with 79 additions and 97 deletions.
62 changes: 29 additions & 33 deletions ctms/backport_legacy_waitlists.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from .schemas import RelayWaitlistInSchema, VpnWaitlistInSchema, WaitlistInSchema


def format_legacy_vpn_relay_waitlist_input(
def format_legacy_vpn_relay_waitlist_input( # noqa: PLR0912,PLR0915
db: Session, email_id: UUID4, input_data, schema_class, metrics: Optional[Dict]
):
"""
Expand Down Expand Up @@ -100,42 +100,38 @@ def format_legacy_vpn_relay_waitlist_input(
to_update.append(
WaitlistInSchema(name=waitlist.name, subscribed=False)
)
else:
# `relay_waitlist` field was specified.
if input_relay_newsletters:
# We are subscribing to a `relay-*-waitlist` newsletter.
# We want to keep the other Relay waitlists already subscribed.
for waitlist in relay_waitlists:
to_update.append(
WaitlistInSchema(
name=waitlist.name,
subscribed=waitlist.subscribed,
fields=waitlist.fields,
)
)
for newsletter in input_relay_newsletters:
name = newsletter["name"].replace("-waitlist", "")
to_update.append(
WaitlistInSchema(
name=name, fields={"geo": parsed_relay.geo}
)
elif input_relay_newsletters:
# We are subscribing to a `relay-*-waitlist` newsletter.
# We want to keep the other Relay waitlists already subscribed.
for waitlist in relay_waitlists:
to_update.append(
WaitlistInSchema(
name=waitlist.name,
subscribed=waitlist.subscribed,
fields=waitlist.fields,
)
elif len(relay_waitlists) == 0:
# We are subscribing to the `relay` waitlist for the first time.
)
for newsletter in input_relay_newsletters:
name = newsletter["name"].replace("-waitlist", "")
to_update.append(
WaitlistInSchema(name="relay", fields={"geo": parsed_relay.geo})
WaitlistInSchema(name=name, fields={"geo": parsed_relay.geo})
)
else:
# `relay_waitlist` was specified but without newsletter, hence update geo field
# of all subscribed Relay waitlists.
for waitlist in relay_waitlists:
to_update.append(
WaitlistInSchema(
name=waitlist.name,
source=waitlist.source,
fields={"geo": parsed_relay.geo},
)
elif len(relay_waitlists) == 0:
# We are subscribing to the `relay` waitlist for the first time.
to_update.append(
WaitlistInSchema(name="relay", fields={"geo": parsed_relay.geo})
)
else:
# `relay_waitlist` was specified but without newsletter, hence update geo field
# of all subscribed Relay waitlists.
for waitlist in relay_waitlists:
to_update.append(
WaitlistInSchema(
name=waitlist.name,
source=waitlist.source,
fields={"geo": parsed_relay.geo},
)
)

if to_update:
formatted["waitlists"] = [wl.model_dump() for wl in to_update]
Expand Down
2 changes: 1 addition & 1 deletion ctms/bin/client_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def print_new_credentials(
)


def main(db, settings, test_args=None):
def main(db, settings, test_args=None): # noqa: PLR0912
"""
Process the command line and create or update client credentials
Expand Down
2 changes: 1 addition & 1 deletion ctms/bin/delete_bulk.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,4 @@ def delete(obj, email, email_file):


if __name__ == "__main__":
cli() # pylint: disable=no-value-for-parameter
cli()
26 changes: 12 additions & 14 deletions ctms/crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def ping(db: Session):
try:
db.execute(text("SELECT 1"))
return True
except Exception as exc: # pylint:disable = broad-exception-caught
except Exception as exc:
logger.exception(exc)
return False

Expand Down Expand Up @@ -119,15 +119,14 @@ def get_all_contacts_from_ids(db, email_ids):

def get_bulk_query(start_time, end_time, after_email_uuid, mofo_relevant):
filters = [
# pylint: disable-next=comparison-with-callable
Email.update_timestamp >= start_time,
Email.update_timestamp < end_time, # pylint: disable=comparison-with-callable
Email.update_timestamp < end_time,
Email.email_id != after_email_uuid,
]
if mofo_relevant is False:
filters.append(
or_(
Email.mofo == None, # pylint: disable=C0121
Email.mofo == None,
Email.mofo.has(mofo_relevant=mofo_relevant),
)
)
Expand Down Expand Up @@ -487,7 +486,7 @@ def _update_orm(orm: Base, update_dict: dict):
setattr(orm, key, value)


def update_contact(
def update_contact( # noqa: PLR0912
db: Session, email: Email, update_data: dict, metrics: Optional[Dict]
) -> None:
"""Update an existing contact using a sparse update dictionary"""
Expand All @@ -510,15 +509,14 @@ def update_contact(
if existing:
db.delete(existing)
setattr(email, group_name, None)
elif existing is None:
new = creator(db, email_id, schema(**update_data[group_name]))
setattr(email, group_name, new)
else:
if existing is None:
new = creator(db, email_id, schema(**update_data[group_name]))
setattr(email, group_name, new)
else:
_update_orm(existing, update_data[group_name])
if schema.model_validate(existing).is_default():
db.delete(existing)
setattr(email, group_name, None)
_update_orm(existing, update_data[group_name])
if schema.model_validate(existing).is_default():
db.delete(existing)
setattr(email, group_name, None)

update_data = format_legacy_vpn_relay_waitlist_input(
db, email_id, update_data, dict, metrics
Expand Down Expand Up @@ -585,7 +583,7 @@ def get_active_api_client_ids(db: Session) -> List[str]:


def update_api_client_last_access(db: Session, api_client: ApiClient):
api_client.last_access = func.now() # pylint: disable=not-callable
api_client.last_access = func.now()
db.add(api_client)


Expand Down
2 changes: 1 addition & 1 deletion ctms/exception_capture.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def init_sentry():
sentry_debug = False

version_info = config.get_version()
# pylint: disable=abstract-class-instantiated

sentry_sdk.init(
release=version_info["version"],
debug=sentry_debug,
Expand Down
5 changes: 2 additions & 3 deletions ctms/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,12 @@


def set_metrics(metrics: Any) -> None:
global METRICS # pylint: disable=global-statement
global METRICS # noqa: PLW0603
METRICS = metrics


# pylint: disable-next=unnecessary-lambda-assignment
get_metrics_registry = lambda: METRICS_REGISTRY
get_metrics = lambda: METRICS # pylint: disable=unnecessary-lambda-assignment
get_metrics = lambda: METRICS

oauth2_scheme = OAuth2ClientCredentials(tokenUrl="token")
token_scheme = HTTPBasic(auto_error=False)
Expand Down
18 changes: 8 additions & 10 deletions ctms/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,31 +24,29 @@ class Base(DeclarativeBase):
pass


class CaseInsensitiveComparator(
Comparator
): # pylint: disable=abstract-method,too-many-ancestors
class CaseInsensitiveComparator(Comparator):
def __eq__(self, other):
return func.lower(self.__clause_element__()) == func.lower(other)


class TimestampMixin:
@declared_attr
def create_timestamp(cls): # pylint: disable=no-self-argument
def create_timestamp(cls):
return mapped_column(
TIMESTAMP(timezone=True),
nullable=False,
server_default=func.now(), # pylint: disable=not-callable
server_default=func.now(),
)

@declared_attr
def update_timestamp(cls): # pylint: disable=no-self-argument
def update_timestamp(cls):
return mapped_column(
TIMESTAMP(timezone=True),
nullable=False,
server_default=func.now(), # pylint: disable=not-callable
server_default=func.now(),
# server_onupdate would be nice to use here, but it's not supported
# by Postgres
onupdate=func.now(), # pylint: disable=not-callable
onupdate=func.now(),
)


Expand Down Expand Up @@ -87,7 +85,7 @@ def primary_email_insensitive(self):
return self.primary_email.lower()

@primary_email_insensitive.comparator
def primary_email_insensitive_comparator(cls): # pylint: disable=no-self-argument
def primary_email_insensitive_comparator(cls):
return CaseInsensitiveComparator(cls.primary_email)

# Indexes
Expand Down Expand Up @@ -158,7 +156,7 @@ def fxa_primary_email_insensitive(self):
@fxa_primary_email_insensitive.comparator
def fxa_primary_email_insensitive_comparator(
cls,
): # pylint: disable=no-self-argument
):
return CaseInsensitiveComparator(cls.primary_email)

# Indexes
Expand Down
6 changes: 3 additions & 3 deletions ctms/routers/contacts.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ def create_ctms_contact(
try:
create_contact(db, email_id, contact, get_metrics())
db.commit()
except Exception as e: # pylint:disable = W0703
except Exception as e:
db.rollback()
if isinstance(e, IntegrityError):
raise HTTPException(status_code=409, detail="Contact already exists") from e
Expand Down Expand Up @@ -258,7 +258,7 @@ def create_or_update_ctms_contact(
try:
create_or_update_contact(db, email_id, contact, get_metrics())
db.commit()
except Exception as e: # pylint:disable = W0703
except Exception as e:
db.rollback()
if isinstance(e, IntegrityError):
raise HTTPException(
Expand Down Expand Up @@ -305,7 +305,7 @@ def partial_update_ctms_contact(

try:
db.commit()
except Exception as e: # pylint:disable = W0703
except Exception as e:
db.rollback()
if isinstance(e, IntegrityError):
raise HTTPException(
Expand Down
2 changes: 1 addition & 1 deletion ctms/schemas/bulk.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def mofo_relevant_must_not_be_blank(cls, value):
after: Optional[str] = Field(default=None, validate_default=True)

@field_validator("after", mode="before")
def after_must_be_base64_decodable(cls, value): # pylint: disable=no-self-argument
def after_must_be_base64_decodable(cls, value):
if value in BLANK_VALS:
return None # Default
try:
Expand Down
4 changes: 2 additions & 2 deletions ctms/schemas/contact.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ def set_default_mofo(cls, value):

@model_validator(mode="before")
@classmethod
def legacy_waitlists(cls, values): # pylint: disable=no-self-argument
def legacy_waitlists(cls, values):
# Show computed fields in response for retro-compatibility.
values["vpn_waitlist"] = VpnWaitlistSchema()
values["relay_waitlist"] = RelayWaitlistSchema()
Expand All @@ -301,7 +301,7 @@ def legacy_waitlists(cls, values): # pylint: disable=no-self-argument
continue
if isinstance(waitlist, dict):
# TODO: figure out why dict from `response_model` decorators param in app.py)
waitlist = WaitlistSchema(**waitlist)
waitlist = WaitlistSchema(**waitlist) # noqa: PLW2901
if waitlist.name == "vpn":
values["vpn_waitlist"] = VpnWaitlistSchema(
geo=waitlist.fields.get("geo"),
Expand Down
6 changes: 3 additions & 3 deletions ctms/schemas/waitlist.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ class WaitlistTableSchema(WaitlistTimestampedSchema):
model_config = ConfigDict(extra="forbid")


def CountryField(): # pylint:disable = invalid-name
def CountryField():
return Field(
default=None,
max_length=100,
Expand All @@ -124,7 +124,7 @@ def CountryField(): # pylint:disable = invalid-name
)


def PlatformField(): # pylint:disable = invalid-name
def PlatformField():
return Field(
default=None,
max_length=100,
Expand All @@ -134,7 +134,7 @@ def PlatformField(): # pylint:disable = invalid-name


def validate_waitlist_newsletters(
contact: Union["ContactInBase", "ContactPatchSchema"]
contact: Union["ContactInBase", "ContactPatchSchema"],
):
"""
This helper validates that when subscribing to `relay-*-waitlist`
Expand Down
3 changes: 0 additions & 3 deletions tests/factories/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@
from ctms.database import ScopedSessionLocal


# Pylint complains that we don't override `evaluate` in the class below, but
# neither does the class that we're subclassing from, hence the disable.
# pylint: disable-next=abstract-method
class RelatedFactoryVariableList(factory.RelatedFactoryList):
"""allows overriding ``size`` during factory usage, e.g. ParentFactory(list_factory__size=4)
Expand Down
Loading

0 comments on commit 402e045

Please sign in to comment.