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

session.commit() and unit tests cleanup #4678

Merged
merged 4 commits into from
Dec 18, 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
3 changes: 0 additions & 3 deletions server/polar/account/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ async def check_review_threshold(
):
account.status = Account.Status.UNDER_REVIEW
session.add(account)
await session.commit()

enqueue_job("account.under_review", account_id=account.id)

Expand All @@ -156,7 +155,6 @@ async def confirm_account_reviewed(
account.next_review_threshold = None

session.add(account)
await session.commit()

enqueue_job("account.reviewed", account_id=account.id)

Expand Down Expand Up @@ -286,7 +284,6 @@ async def update_account_from_stripe(
account.status = Account.Status.ONBOARDING_STARTED

session.add(account)
await session.commit()

return account

Expand Down
4 changes: 2 additions & 2 deletions server/polar/benefit/service/benefit_grant.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ async def grant_benefit(
grant.set_granted()

session.add(grant)
await session.commit()
await session.flush()

await eventstream_publish(
"benefit.granted",
Expand Down Expand Up @@ -230,7 +230,7 @@ async def revoke_benefit(
grant.set_revoked()

session.add(grant)
await session.commit()
await session.flush()

await eventstream_publish(
"benefit.revoked",
Expand Down
2 changes: 1 addition & 1 deletion server/polar/integrations/github/service/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ async def create(
)

session.add(new_user)
await session.commit()
await session.flush()

log.info("github.user.create", user_id=new_user.id, username=github_user.login)

Expand Down
4 changes: 2 additions & 2 deletions server/polar/integrations/stripe/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ async def get_or_create_user_customer(
.values(stripe_customer_id=customer.id)
)
await session.execute(stmt)
await session.commit()
await session.flush()

return customer

Expand Down Expand Up @@ -278,7 +278,7 @@ async def get_or_create_org_customer(
.values(stripe_customer_id=customer.id)
)
await session.execute(stmt)
await session.commit()
await session.flush()

return customer

Expand Down
2 changes: 1 addition & 1 deletion server/polar/kit/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ async def soft_delete(self, session: AsyncSession, id: UUID) -> None:
)
)
await session.execute(stmt)
await session.commit()
await session.flush()


class ResourceService(
Expand Down
2 changes: 0 additions & 2 deletions server/polar/magic_link/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ async def request(

magic_link = MagicLink(**magic_link_create.model_dump(exclude_unset=True))
session.add(magic_link)
await session.commit()

return magic_link, token

Expand Down Expand Up @@ -127,7 +126,6 @@ async def authenticate(
async def delete_expired(self, session: AsyncSession) -> None:
statement = delete(MagicLink).where(MagicLink.expires_at < utc_now())
await session.execute(statement)
await session.commit()

async def _get_valid_magic_link_by_token_hash(
self, session: AsyncSession, token_hash: str
Expand Down
5 changes: 2 additions & 3 deletions server/polar/notifications/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ async def send_to_user(
)

session.add(notification)
await session.commit()
await session.flush()
enqueue_job("notifications.send", notification_id=notification.id)
return True

Expand Down Expand Up @@ -95,7 +95,7 @@ async def send_to_anonymous_email(
)

session.add(notification)
await session.commit()
await session.flush()
enqueue_job("notifications.send", notification_id=notification.id)

async def send_to_pledger(
Expand Down Expand Up @@ -155,7 +155,6 @@ async def set_user_last_read(
)
)
await session.execute(stmt)
await session.commit()


notifications = NotificationsService()
101 changes: 0 additions & 101 deletions server/polar/sentry.py
Original file line number Diff line number Diff line change
@@ -1,114 +1,14 @@
import os
from typing import TYPE_CHECKING, Self

import posthog as global_posthog
import sentry_sdk
from posthog.request import DEFAULT_HOST
from sentry_sdk.hub import Hub
from sentry_sdk.integrations import Integration
from sentry_sdk.integrations.fastapi import FastApiIntegration
from sentry_sdk.scope import add_global_event_processor
from sentry_sdk.utils import Dsn

from polar.auth.models import AuthSubject, Subject, is_user
from polar.config import settings
from polar.posthog import posthog

if TYPE_CHECKING:
from posthog import Posthog
from sentry_sdk import _types

POSTHOG_ID_TAG = "posthog_distinct_id"


class PostHogIntegration(Integration):
"""
PostHog integration for Sentry, heavily pulled from the official one from PostHog.

https://github.com/PostHog/posthog-python/blob/master/posthog/sentry/posthog_integration.py

The official one suffers from a few limitations:

* Doesn't have a clean way to set dynamic parameters, like `organization`.
* Doesn't support a custom instance of PostHog, only the global one.
* Tries to serialize the Sentry exception object, but fails to do so.

This implementation tries to solve those limitations to fit our use-case.
"""

identifier = "posthog-python"

def __init__(
self,
posthog: "Posthog | None" = None,
organization: str | None = None,
project_id: str | None = None,
prefix: str = "https://sentry.io/organizations/",
):
self.posthog = posthog if posthog else global_posthog
self.organization = organization
self.project_id = project_id
self.prefix = prefix

@staticmethod
def setup_once() -> None:
@add_global_event_processor
def processor(event: "_types.Event", hint: "_types.Hint") -> "_types.Event":
integration: Self | None = Hub.current.get_integration(PostHogIntegration)
if integration is not None:
if event.get("level") != "error":
return event

if event.get("tags", {}).get(POSTHOG_ID_TAG):
posthog = integration.posthog

posthog_distinct_id = event["tags"][POSTHOG_ID_TAG]

# Posthog and Module("Posthog") are not compatible types in Python 3.12 / Mypy 1.7
# Adding if/else here as a workaround
host = (
posthog.host
if posthog and isinstance(posthog, Posthog)
else global_posthog.host
)

event["tags"]["PostHog URL"] = (
f"{host or DEFAULT_HOST}/person/{posthog_distinct_id}"
)

properties = {
"$sentry_event_id": event["event_id"],
}

if integration.organization:
project_id = integration.project_id
if project_id is None:
sentry_client = Hub.current.client
if (
sentry_client is not None
and sentry_client.dsn is not None
):
project_id = Dsn(sentry_client.dsn).project_id

if project_id:
properties["$sentry_url"] = (
f"{integration.prefix}{integration.organization}"
"/issues/"
f"?project={project_id}&query={event['event_id']}"
)

# Posthog and Module("Posthog") are not compatible types in Python 3.12 / Mypy 1.7
# Adding if/else here as a workaround
if posthog and isinstance(posthog, Posthog):
posthog.capture(posthog_distinct_id, "$exception", properties)
else:
global_posthog.capture(
posthog_distinct_id, "$exception", properties
)

return event


def configure_sentry() -> None:
sentry_sdk.init(
dsn=settings.SENTRY_DSN,
Expand All @@ -119,7 +19,6 @@ def configure_sentry() -> None:
server_name=os.environ.get("RENDER_INSTANCE_ID", "localhost"),
environment=settings.ENV,
integrations=[
PostHogIntegration(posthog=posthog.client, organization="polar-sh"),
FastApiIntegration(transaction_style="endpoint"),
],
)
Expand Down
1 change: 0 additions & 1 deletion server/polar/user/service/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ async def set_account(

user.account = account
session.add(user)
await session.commit()
return user

async def link_customers(self, session: AsyncSession, user: User) -> None:
Expand Down
1 change: 0 additions & 1 deletion server/polar/user_organization/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ async def remove_member(
.values(deleted_at=utc_now())
)
await session.execute(stmt)
await session.commit()

def _get_list_by_user_id_query(
self, user_id: UUID, ordered: bool = True
Expand Down
3 changes: 0 additions & 3 deletions server/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,6 @@ pre_deploy = { cmd = "task db_migrate", help = "Pre-deploy command run by Render
[tool.pytest.ini_options]
markers = [
"auth",
"http_auto_expunge",
"skip_db_asserts",
"override_current_user",
"email_order_confirmation",
"email_subscription_confirmation",
]
Expand Down
9 changes: 0 additions & 9 deletions server/tests/account/test_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@


@pytest.mark.asyncio
@pytest.mark.http_auto_expunge
@pytest.mark.auth
async def test_create_invalid_account_type(client: AsyncClient) -> None:
response = await client.post(
Expand All @@ -39,7 +38,6 @@ async def test_create_invalid_account_type(client: AsyncClient) -> None:

@pytest.mark.asyncio
@pytest.mark.parametrize("slug", [None, ""])
@pytest.mark.http_auto_expunge
@pytest.mark.auth
async def test_create_open_collective_missing_slug(
slug: str | None, client: AsyncClient
Expand All @@ -57,7 +55,6 @@ async def test_create_open_collective_missing_slug(


@pytest.mark.asyncio
@pytest.mark.http_auto_expunge
@pytest.mark.parametrize(
"error",
[
Expand Down Expand Up @@ -88,7 +85,6 @@ async def test_create_open_collective_get_collective_error(


@pytest.mark.asyncio
@pytest.mark.http_auto_expunge
@pytest.mark.parametrize(
"collective",
[
Expand Down Expand Up @@ -137,7 +133,6 @@ async def test_create_open_collective_not_eligible(


@pytest.mark.asyncio
@pytest.mark.http_auto_expunge
@pytest.mark.auth
async def test_create_open_collective(
session: AsyncSession, mocker: MockerFixture, client: AsyncClient
Expand Down Expand Up @@ -173,7 +168,6 @@ async def test_create_open_collective(


@pytest.mark.asyncio
@pytest.mark.http_auto_expunge
@pytest.mark.auth
async def test_create_personal_stripe(
user: User,
Expand Down Expand Up @@ -211,7 +205,6 @@ def to_dict(self) -> dict[str, Any]:


@pytest.mark.asyncio
@pytest.mark.http_auto_expunge
@pytest.mark.auth
async def test_onboarding_link_open_collective(
open_collective_account: Account, client: AsyncClient
Expand All @@ -225,7 +218,6 @@ async def test_onboarding_link_open_collective(


@pytest.mark.asyncio
@pytest.mark.http_auto_expunge
@pytest.mark.auth
async def test_dashboard_link_not_existing_account(
user: User,
Expand All @@ -241,7 +233,6 @@ async def test_dashboard_link_not_existing_account(


@pytest.mark.asyncio
@pytest.mark.http_auto_expunge
@pytest.mark.auth
async def test_dashboard_link_open_collective(
open_collective_account: Account, client: AsyncClient
Expand Down
1 change: 0 additions & 1 deletion server/tests/benefit/benefits/test_ads.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@


@pytest.mark.asyncio
@pytest.mark.skip_db_asserts
async def test_grant(
session: AsyncSession,
redis: Redis,
Expand Down
1 change: 0 additions & 1 deletion server/tests/benefit/benefits/test_downloadables.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@


@pytest.mark.asyncio
@pytest.mark.skip_db_asserts
class TestDownloadblesBenefit:
@pytest.mark.auth
async def test_grant_one(
Expand Down
1 change: 0 additions & 1 deletion server/tests/benefit/service/test_benefit_grant.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,6 @@ async def test_several_benefit_grants_should_individual_revoke(


@pytest.mark.asyncio
@pytest.mark.skip_db_asserts
class TestEnqueueBenefitsGrants:
@pytest.mark.parametrize("task", ["grant", "revoke"])
async def test_subscription_scope(
Expand Down
Loading
Loading