-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add type safety and type hinting #317
base: master
Are you sure you want to change the base?
Changes from 10 commits
4950ade
7bf5144
fc30c55
2264a3b
676b65a
1d2d912
3f5fd46
0df3281
4d69e42
848c72b
43daeaa
9e709f5
c0bffdb
5232436
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import datetime | ||
import json | ||
from typing import Any | ||
|
||
import requests | ||
from django.conf import settings | ||
|
@@ -16,14 +17,14 @@ | |
|
||
|
||
class DiningAPIWrapper: | ||
def __init__(self): | ||
def __init__(self) -> None: | ||
self.token = None | ||
self.expiration = timezone.localtime() | ||
self.openid_endpoint = ( | ||
"https://sso.apps.k8s.upenn.edu/auth/realms/master/protocol/openid-connect/token" | ||
) | ||
|
||
def update_token(self): | ||
def update_token(self) -> None: | ||
if self.expiration > timezone.localtime(): | ||
return | ||
body = { | ||
|
@@ -33,11 +34,11 @@ def update_token(self): | |
} | ||
response = requests.post(self.openid_endpoint, data=body).json() | ||
if "error" in response: | ||
raise APIError(f"Dining: {response['error']}, {response.get('error_description')}") | ||
raise APIError(f"Dining: {response['error']}, {response.get('error_description', '')}") | ||
self.expiration = timezone.localtime() + datetime.timedelta(seconds=response["expires_in"]) | ||
self.token = response["access_token"] | ||
|
||
def request(self, *args, **kwargs): | ||
def request(self, *args: Any, **kwargs: Any) -> requests.Response: | ||
"""Make a signed request to the dining API.""" | ||
self.update_token() | ||
|
||
|
@@ -54,7 +55,7 @@ def request(self, *args, **kwargs): | |
except (ConnectTimeout, ReadTimeout, ConnectionError): | ||
raise APIError("Dining: Connection timeout") | ||
|
||
def get_venues(self): | ||
def get_venues(self) -> list[dict[str, Any]]: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we define the |
||
results = [] | ||
venues_route = OPEN_DATA_ENDPOINTS["VENUES"] | ||
response = self.request("GET", venues_route) | ||
|
@@ -107,7 +108,7 @@ def get_venues(self): | |
results.append(value) | ||
return results | ||
|
||
def load_menu(self, date=timezone.now().date()): | ||
def load_menu(self, date: datetime.date = timezone.now().date()) -> None: | ||
""" | ||
Loads the weeks menu starting from today | ||
NOTE: This method should only be used in load_next_menu.py, which is | ||
|
@@ -147,7 +148,9 @@ def load_menu(self, date=timezone.now().date()): | |
# Append stations to dining menu | ||
self.load_stations(daypart["stations"], dining_menu) | ||
|
||
def load_stations(self, station_response, dining_menu): | ||
def load_stations( | ||
self, station_response: list[dict[str, Any]], dining_menu: DiningMenu | ||
) -> None: | ||
for station_data in station_response: | ||
# TODO: This is inefficient for venues such as Houston Market | ||
station = DiningStation.objects.create(name=station_data["label"], menu=dining_menu) | ||
|
@@ -158,7 +161,7 @@ def load_stations(self, station_response, dining_menu): | |
station.items.add(*items) | ||
station.save() | ||
|
||
def load_items(self, item_response): | ||
def load_items(self, item_response: dict[str, Any]) -> None: | ||
item_list = [ | ||
DiningItem( | ||
item_id=key, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import datetime | ||
from typing import Any | ||
|
||
from django.core.management.base import BaseCommand | ||
from django.utils import timezone | ||
|
@@ -13,7 +14,7 @@ class Command(BaseCommand): | |
the next 7 days, including the original date. | ||
""" | ||
|
||
def handle(self, *args, **kwargs): | ||
def handle(self, *args: Any, **kwargs: Any) -> None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would setting these types explicitly break anything? |
||
d = DiningAPIWrapper() | ||
d.load_menu(timezone.now().date() + datetime.timedelta(days=6)) | ||
self.stdout.write("Loaded new Dining Menu!") |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import json | ||
from typing import Any | ||
|
||
from rest_framework import serializers | ||
|
||
|
@@ -18,7 +19,7 @@ class Meta: | |
model = DiningItem | ||
fields = "__all__" | ||
|
||
def get_nutrition_info(self, obj): | ||
def get_nutrition_info(self, obj: DiningItem) -> dict[str, Any]: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similar question here, if we are explicit about the type, does that break anything? |
||
try: | ||
return json.loads(obj.nutrition_info) | ||
except json.JSONDecodeError: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we would want schemas here? Ideally there are built in ones since these are methods we just override?