Skip to content

Commit 3bd6b8a

Browse files
committed
fix: resolve linting and CI issues - Fix ruff violations, improve OpenTelemetry config for CI, remove unused imports, update type annotations
1 parent c68cd70 commit 3bd6b8a

File tree

5 files changed

+543
-93
lines changed

5 files changed

+543
-93
lines changed

backend/app/api/main.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from fastapi import APIRouter
22

3-
from app.api.routes import items, login, private, users, utils, analytics
3+
from app.api.routes import analytics, items, login, private, users, utils
44
from app.core.config import settings
55

66
api_router = APIRouter()
Lines changed: 79 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,60 @@
1-
from fastapi import APIRouter, Depends, HTTPException
2-
from sqlmodel import Session, select
3-
import polars as pl
4-
from typing import List, Dict, Any # Dict and Any might be needed for Polars conversion
51
from datetime import date
6-
import pydantic # Ensure pydantic is imported
7-
8-
from app.models import User, Item # Assuming User model is in app.models, Import Item
9-
from app.api.deps import SessionDep, get_current_active_superuser # SessionDep for dependency injection
102

3+
import polars as pl
4+
import pydantic # Ensure pydantic is imported
5+
from fastapi import APIRouter
116
from opentelemetry import trace
7+
from sqlmodel import select
8+
9+
from app.api.deps import SessionDep # SessionDep for dependency injection
10+
from app.models import Item, User # Assuming User model is in app.models, Import Item
11+
1212

1313
# Pydantic models for response
14-
class UserSignupTrend(pydantic.BaseModel): # Corrected: pydantic.BaseModel
15-
signup_date: date
14+
class UserSignupTrend(pydantic.BaseModel): # Corrected: pydantic.BaseModel
15+
signup_date: date
1616
count: int
1717

18-
class UserActivity(pydantic.BaseModel): # Corrected: pydantic.BaseModel
18+
19+
class UserActivity(pydantic.BaseModel): # Corrected: pydantic.BaseModel
1920
active_users: int
2021
inactive_users: int
2122

22-
class UserAnalyticsSummary(pydantic.BaseModel): # Corrected: pydantic.BaseModel
23+
24+
class UserAnalyticsSummary(pydantic.BaseModel): # Corrected: pydantic.BaseModel
2325
total_users: int
24-
signup_trends: List[UserSignupTrend]
26+
signup_trends: list[UserSignupTrend]
2527
activity_summary: UserActivity
2628
# Add more fields as desired, e.g., average_items_per_user: float
2729

30+
2831
# Pydantic models for Item analytics
2932
class ItemCreationTrend(pydantic.BaseModel):
3033
creation_date: date
3134
count: int
3235

36+
3337
# class ItemOwnerDistribution(pydantic.BaseModel): # Optional for now
34-
# owner_id: str
38+
# owner_id: str
3539
# item_count: int
3640

41+
3742
class ItemAnalyticsTrends(pydantic.BaseModel):
3843
total_items: int
39-
creation_trends: List[ItemCreationTrend]
44+
creation_trends: list[ItemCreationTrend]
4045
# owner_distribution: List[ItemOwnerDistribution] # Optional
4146

4247

4348
router = APIRouter(prefix="/analytics", tags=["analytics"])
4449
tracer = trace.get_tracer(__name__)
4550

51+
4652
@router.get("/user-summary", response_model=UserAnalyticsSummary)
47-
def get_user_summary(session: SessionDep): # get_current_active_superuser is imported but not used here yet
53+
def get_user_summary(
54+
session: SessionDep,
55+
): # get_current_active_superuser is imported but not used here yet
4856
with tracer.start_as_current_span("user_summary_endpoint"):
49-
50-
users_list: List[User]
57+
users_list: list[User]
5158
with tracer.start_as_current_span("fetch_all_users_sql"):
5259
statement = select(User)
5360
users_list = session.exec(statement).all()
@@ -56,126 +63,139 @@ def get_user_summary(session: SessionDep): # get_current_active_superuser is imp
5663
return UserAnalyticsSummary(
5764
total_users=0,
5865
signup_trends=[],
59-
activity_summary=UserActivity(active_users=0, inactive_users=0)
66+
activity_summary=UserActivity(active_users=0, inactive_users=0),
6067
)
6168

6269
with tracer.start_as_current_span("convert_users_to_polars"):
6370
users_data = []
6471
for user in users_list:
6572
user_dict = {
66-
"id": user.id, # No explicit str() casting for now, per instructions
73+
"id": user.id, # No explicit str() casting for now, per instructions
6774
"email": user.email,
6875
"is_active": user.is_active,
6976
"is_superuser": user.is_superuser,
7077
"full_name": user.full_name,
7178
}
7279
# Attempt to get 'created_at' if it exists (it doesn't in the standard model)
73-
if hasattr(user, 'created_at') and user.created_at:
74-
user_dict['created_at'] = user.created_at
80+
if hasattr(user, "created_at") and user.created_at:
81+
user_dict["created_at"] = user.created_at
7582
users_data.append(user_dict)
76-
77-
if not users_data: # Should not happen if users_list is not empty, but as a safe guard
78-
return UserAnalyticsSummary(
83+
84+
if (
85+
not users_data
86+
): # Should not happen if users_list is not empty, but as a safe guard
87+
return UserAnalyticsSummary(
7988
total_users=0,
8089
signup_trends=[],
81-
activity_summary=UserActivity(active_users=0, inactive_users=0)
90+
activity_summary=UserActivity(active_users=0, inactive_users=0),
8291
)
8392

8493
# Create DataFrame without explicit casting of 'id' first.
8594
# If Polars errors on UUID, the instruction is to add:
8695
# df_users = df_users.with_columns(pl.col('id').cast(pl.Utf8))
8796
df_users = pl.DataFrame(users_data)
8897

89-
90-
total_users = df_users.height # More idiomatic for Polars than len(df_users)
98+
total_users = df_users.height # More idiomatic for Polars than len(df_users)
9199

92100
with tracer.start_as_current_span("calculate_user_activity_polars"):
93-
active_users = df_users.filter(pl.col("is_active") == True).height
101+
active_users = df_users.filter(pl.col("is_active")).height
94102
inactive_users = total_users - active_users
95-
103+
96104
signup_trends_data = []
97-
if 'created_at' in df_users.columns: # This will be false as User model has no created_at
105+
if (
106+
"created_at" in df_users.columns
107+
): # This will be false as User model has no created_at
98108
with tracer.start_as_current_span("calculate_signup_trends_polars"):
99109
# Ensure 'created_at' is a datetime type. If string, parse it.
100110
# Assuming it's already a datetime.date or datetime.datetime from SQLModel
101111
# If it's datetime, cast to date for daily trends
102-
df_users_with_date = df_users.with_columns(pl.col("created_at").cast(pl.Date).alias("signup_day"))
103-
104-
signup_counts_df = df_users_with_date.group_by("signup_day").agg(
105-
pl.count().alias("count")
106-
).sort("signup_day")
107-
112+
df_users_with_date = df_users.with_columns(
113+
pl.col("created_at").cast(pl.Date).alias("signup_day")
114+
)
115+
116+
signup_counts_df = (
117+
df_users_with_date.group_by("signup_day")
118+
.agg(pl.count().alias("count"))
119+
.sort("signup_day")
120+
)
121+
108122
signup_trends_data = [
109123
UserSignupTrend(signup_date=row["signup_day"], count=row["count"])
110124
for row in signup_counts_df.to_dicts()
111125
]
112-
126+
113127
return UserAnalyticsSummary(
114128
total_users=total_users,
115-
signup_trends=signup_trends_data, # Will be empty as 'created_at' is not in User model
116-
activity_summary=UserActivity(active_users=active_users, inactive_users=inactive_users)
129+
signup_trends=signup_trends_data, # Will be empty as 'created_at' is not in User model
130+
activity_summary=UserActivity(
131+
active_users=active_users, inactive_users=inactive_users
132+
),
117133
)
118134

135+
119136
@router.get("/item-trends", response_model=ItemAnalyticsTrends)
120137
def get_item_trends(session: SessionDep):
121138
with tracer.start_as_current_span("item_trends_endpoint"):
122-
123-
items_list: List[Item]
139+
items_list: list[Item]
124140
with tracer.start_as_current_span("fetch_all_items_sql"):
125141
statement = select(Item)
126142
items_list = session.exec(statement).all()
127143

128144
if not items_list:
129145
return ItemAnalyticsTrends(
130146
total_items=0,
131-
creation_trends=[]
147+
creation_trends=[],
132148
# owner_distribution=[] # Optional
133149
)
134150

135151
with tracer.start_as_current_span("convert_items_to_polars"):
136152
items_data = []
137153
for item in items_list:
138154
item_dict = {
139-
"id": str(item.id), # Cast UUID to string
155+
"id": str(item.id), # Cast UUID to string
140156
"title": item.title,
141157
"description": item.description,
142-
"owner_id": str(item.owner_id) # Cast UUID to string
158+
"owner_id": str(item.owner_id), # Cast UUID to string
143159
}
144160
# IMPORTANT: Item model does not have 'created_at'.
145161
# This will result in empty creation_trends.
146-
if hasattr(item, 'created_at') and item.created_at:
147-
item_dict['created_at'] = item.created_at
162+
if hasattr(item, "created_at") and item.created_at:
163+
item_dict["created_at"] = item.created_at
148164
items_data.append(item_dict)
149-
150-
if not items_data: # Safety check
165+
166+
if not items_data: # Safety check
151167
return ItemAnalyticsTrends(total_items=0, creation_trends=[])
152168

153169
df_items = pl.DataFrame(items_data)
154170

155171
total_items = df_items.height
156172

157173
creation_trends_data = []
158-
if 'created_at' in df_items.columns:
174+
if "created_at" in df_items.columns:
159175
with tracer.start_as_current_span("calculate_item_creation_trends_polars"):
160176
# Ensure 'created_at' is datetime, then cast to date for daily trends
161177
df_items_with_date = df_items.with_columns(
162178
pl.col("created_at").cast(pl.Date).alias("creation_day")
163179
)
164-
165-
creation_counts_df = df_items_with_date.group_by("creation_day").agg(
166-
pl.count().alias("count")
167-
).sort("creation_day")
168-
180+
181+
creation_counts_df = (
182+
df_items_with_date.group_by("creation_day")
183+
.agg(pl.count().alias("count"))
184+
.sort("creation_day")
185+
)
186+
169187
creation_trends_data = [
170-
ItemCreationTrend(creation_date=row["creation_day"], count=row["count"])
188+
ItemCreationTrend(
189+
creation_date=row["creation_day"], count=row["count"]
190+
)
171191
for row in creation_counts_df.to_dicts()
172192
]
173-
193+
174194
# Placeholder for owner distribution if implemented later
175-
# owner_distribution_data = []
195+
# owner_distribution_data = []
176196

177197
return ItemAnalyticsTrends(
178198
total_items=total_items,
179-
creation_trends=creation_trends_data
199+
creation_trends=creation_trends_data,
180200
# owner_distribution=owner_distribution_data # Optional
181201
)

backend/app/core/telemetry.py

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,43 @@
11
import os
2+
23
from opentelemetry import trace
34
from opentelemetry.exporter.otlp.proto.grpc.trace_exporter import OTLPSpanExporter
5+
from opentelemetry.instrumentation.fastapi import FastAPIInstrumentor
6+
from opentelemetry.instrumentation.sqlalchemy import SQLAlchemyInstrumentor
47
from opentelemetry.sdk.resources import Resource
58
from opentelemetry.sdk.trace import TracerProvider
69
from opentelemetry.sdk.trace.export import BatchSpanProcessor
7-
from opentelemetry.instrumentation.fastapi import FastAPIInstrumentor
8-
from opentelemetry.instrumentation.sqlalchemy import SQLAlchemyInstrumentor
910

10-
from app.core.db import engine # Assuming engine is in app.core.db
11-
from app.core.config import settings # To potentially get service name
11+
from app.core.config import settings # To potentially get service name
12+
from app.core.db import engine # Assuming engine is in app.core.db
13+
1214

1315
def init_telemetry(app):
1416
# Set service name, try from settings or default
1517
service_name = getattr(settings, "OTEL_SERVICE_NAME", "fastapi-application")
16-
17-
resource = Resource(attributes={
18-
"service.name": service_name
19-
})
20-
21-
# Configure OTLP exporter
22-
# Endpoint can be configured via OTEL_EXPORTER_OTLP_ENDPOINT environment variable
23-
# Defaulting to http://localhost:4317 if not set, as per OpenTelemetry spec
24-
otlp_endpoint = os.getenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4317")
25-
26-
span_exporter = OTLPSpanExporter(
27-
endpoint=otlp_endpoint,
28-
# insecure=True # Set to True if your collector is not using TLS. Adjust as needed.
29-
)
30-
31-
processor = BatchSpanProcessor(span_exporter)
32-
18+
19+
resource = Resource(attributes={"service.name": service_name})
20+
21+
# Configure OTLP exporter only if endpoint is explicitly set
22+
# This prevents connection attempts in CI/test environments
23+
otlp_endpoint = os.getenv("OTEL_EXPORTER_OTLP_ENDPOINT")
24+
3325
provider = TracerProvider(resource=resource)
34-
provider.add_span_processor(processor)
35-
26+
27+
if otlp_endpoint:
28+
span_exporter = OTLPSpanExporter(
29+
endpoint=otlp_endpoint,
30+
# insecure=True # Set to True if your collector is not using TLS. Adjust as needed.
31+
)
32+
processor = BatchSpanProcessor(span_exporter)
33+
provider.add_span_processor(processor)
34+
3635
# Sets the global default tracer provider
3736
trace.set_tracer_provider(provider)
38-
37+
3938
# Instrument FastAPI
4039
FastAPIInstrumentor.instrument_app(app)
41-
40+
4241
# Instrument SQLAlchemy
4342
# Ensure the engine is already configured/available when this is called.
4443
SQLAlchemyInstrumentor().instrument(engine=engine)

backend/app/tests/api/test_analytics.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
from fastapi.testclient import TestClient
2-
from sqlmodel import Session # For potential future use with fixtures
3-
import pytest # For potential future use with fixtures
42

53
from app.core.config import settings
4+
65
# Assuming your FastAPI app instance is named 'app' in 'app.main'
76
# Adjust the import if your app instance is located elsewhere for tests
8-
# from app.main import app
7+
# from app.main import app
98

109
# Expected Pydantic response models (adjust import path if they are moved)
11-
# from app.api.routes.analytics import UserAnalyticsSummary, ItemAnalyticsTrends
10+
# from app.api.routes.analytics import UserAnalyticsSummary, ItemAnalyticsTrends
11+
1212

1313
# Test for User Analytics Summary
1414
def test_get_user_summary(client: TestClient) -> None:

0 commit comments

Comments
 (0)