NHL Stats web app

Question

There is a web app. My first attempt to use NHL API have been reviewed here. I'm interested in any kind of refactoring of this code. But I have some questions that I want to point at specifically.

First of all, is this a right approach to have all this dirty work for a 'views' in a 'services.py' file? I started with just putting all of these methods in the 'views', but it gets really messy and it was obvious that I should have made views simpler.

The more fundamental question is optimization for performance. There is a lot of loops going on just to prepare two lists for 'players' template. And then two loops(~85 and 800+items) to make tables shown. And top this with some JS scrips. Sorting, paginating, parsing, auto numbered column. So the page is REALLY slow. I couldn't see that I could make it much faster without totally changed approach. Am I right?

I see the way to make it faster. Have a scheduler to scrape all of this stats and write it to the DB. Now I have a command to write info only to 'name' and 'nhl_id' fields of the Player model. Didn't want to depend so much on DB and tried to fetch most of the info from API directly. Because I've thought I could manage to get all of the needed stats from one endpoint.

Individual player pages loading fast enough, but there is another problem. I couldn't find the endpoint with FW stats present (individual endpoint for a player). This also can be solved if I had all of the stats in the model (there is FW stat in the endpoint with a list of all NHL players combined)

views.py

import requests
from django.http import HttpResponse, HttpResponseRedirect
from django.shortcuts import render

from . import services
from .models import Player


def home(request):
    return render(request, 'players/home.html', {'title': 'Home'})


def about(request):
    return render(request, 'players/about.html', {'title': 'About'})


def players(request):
    context = {
        'goalies': services.goalies_list(request),
        'skaters': services.skaters_list(request),
    }
    return render(request, 'players/players.html', context)


def search(request):
    if 'q' in request.GET and request.GET['q']:
        q = request.GET['q']
        result = Player.objects.filter(name__icontains=q)
        result_list = result.values()
        for item in result_list:
            if services.is_favorite(request, item['nhl_id']):
                item['is_favorite'] = True

        context = {
            'players': result_list,
            'query': q,
        }

        return render(request, 'players/search_results.html', context)
    else:
        return HttpResponse('Please submit a search term.')


def player_detail(request, slug, nhl_id):
    bio = services.season_stats(nhl_id)
    context = {
        'is_favorite': services.is_favorite(request, nhl_id),
        'player': services.get_player(nhl_id),
        'bio': bio,
        'stats': bio['stats'][0]['splits'][0]['stat'],
        'total': services.career_stats(nhl_id),
        'sbs_stats': services.sbs_stats(nhl_id),
        'last_gms': services.gamelog(nhl_id)[:5],
        'countries': services.COUNTRIES,
        'team': services.TEAM_ABBR,
    }

    return render(request, 'players/player_detail.html', context)


def player_gamelog(request, slug, nhl_id):
    context = {
        'bio': services.season_stats(nhl_id),
        'gamelog': services.gamelog(nhl_id),
        'team': services.TEAM_ABBR,
    }
    return render(request, 'players/player_gamelog.html', context)


def player_favorite(request, slug, nhl_id):
    player = services.get_player(nhl_id)
    if player.favorite.filter(id=request.user.id).exists():
        player.favorite.remove(request.user)
    else:
        player.favorite.add(request.user)
    return HttpResponseRedirect(request.META.get('HTTP_REFERER', '/'))


def favorites(request):
    user = request.user
    favorites = user.favorite.all()

    context = {
        'favorites': favorites,
        }

    return render(request, 'players/favorites.html', context)


def teams(request):
    response = requests.get(services.URL_TEAMS.format(''))
    context = {
        'teams': response.json()['teams'],
    }
    return render(request, 'players/teams.html', context)


# 'Defencemen', 'Forwards', 'tab2', 'tab3' should probably be declared as a constants
def team_detail(request, slug, team_id):
    response = requests.get(services.URL_TEAMS.format(team_id))
    rost = services.roster(request, slug, team_id)
    context = {
        'goalies': services.roster_position(request, rost, services.POSITIONS[0],
                                            slug, team_id),
        'skaters': [
            {
                'type': 'Defencemen',
                'list': services.roster_position(request, rost, services.POSITIONS[1],
                                                 slug, team_id),
                'table_id': 'tab6',
            },
            {
                'type': 'Forwards',
                'list': services.roster_position(request, rost, services.POSITIONS[2:],
                                                 slug, team_id),
                'table_id': 'tab7',
            }
            ],
        'team': response.json()['teams'][0],
    }

    return render(request, 'players/team_detail.html', context)

services.py

import datetime

import requests
from django.shortcuts import get_object_or_404

from .models import Player


URL = 'http://www.nhl.com/stats/rest/{}'
PL_STAT_URL_BASE = 'https://statsapi.web.nhl.com/api/v1/people/'
URL_TEAMS = 'https://statsapi.web.nhl.com/api/v1/teams/{}'

PL_TYPE1 = 'goalies'
PL_TYPE2 = 'skaters'
POSITIONS = ['G', 'D', 'C', 'LW', 'RW', 'L', 'R']
WING = 'W'
NHL = 'National Hockey League'
REP_TYPE1 = 'goaliesummary'
REP_TYPE2 = 'skatersummary'
REP_TYPE3 = 'realtime'
REP_TYPE4 = 'timeonice'
STAT_TYPE1 = 'statsSingleSeason'
STAT_TYPE2 = 'yearByYear'
STAT_TYPE3 = 'careerRegularSeason'
STAT_TYPE4 = 'gameLog'

COUNTRIES = {
    'RUS': 'Russia',
    'CAN': 'Canada',
    'USA': 'USA',
    'CZE': 'Czech Republic',
    'CHE': 'Switzerland',
    'SWE': 'Sweden',
    'FIN': 'Finland',
    'DEU': 'Germany',
    'DNK': 'Denmark',
    'AUT': 'Austria',
    'LVA': 'Latvia',
    'SVN': 'Slovenia',
    'SVK': 'Slovakia',
    'NLD': 'Netherlands',
    'AUS': 'Australia',
    'GBR': 'Great Britain',
}

TEAM_ABBR = {
    1: 'NJD',
    2: 'NYI',
    3: 'NYR',
    4: 'PHI',
    5: 'PIT',
    6: 'BOS',
    7: 'BUF',
    8: 'MTL',
    9: 'OTT',
    10: 'TOR',
    11: '',
    12: 'CAR',
    13: 'FLA',
    14: 'TBL',
    15: 'WSH',
    16: 'CHI',
    17: 'DET',
    18: 'NSH',
    19: 'STL',
    20: 'CGY',
    21: 'COL',
    22: 'EDM',
    23: 'VAN',
    24: 'ANA',
    25: 'DAL',
    26: 'LAK',
    27: '',
    28: 'SJS',
    29: 'CBJ',
    30: 'MIN',
    52: 'WPG',
    53: 'ARI',
    54: 'VGK',
}


HEIGHT_CONVERT = {
    65: "5' 5\"",
    66: "5' 6\"",
    67: "5' 7\"",
    68: "5' 8\"",
    69: "5' 9\"",
    70: "5' 10\"",
    71: "5' 11\"",
    72: "6' 0\"",
    73: "6' 1\"",
    74: "6' 2\"",
    75: "6' 3\"",
    76: "6' 4\"",
    77: "6' 5\"",
    78: "6' 6\"",
    79: "6' 7\"",
    80: "6' 8\"",
    81: "6' 9\"",
    82: "6' 10\"",
}


def time_from_sec(time):
    min_, sec = divmod(time, 60)
    min_ = int(min_)
    sec = str(int(sec)).zfill(2)
    return f'{min_}:{sec}'.rjust(5, '0')


def time_to_sec(time):
    min_sec = time.split(':')
    return int(min_sec[0])*60 + int(min_sec[1])


def players_resp(rep_type, pl_type):
    params = {
        'isAggregate': 'false',
        'reportType': 'season',
        'isGame': 'false',
        'reportName': rep_type,
        'cayenneExp': 'gameTypeId=2 and seasonId=20182019',
    }

    return requests.get(URL.format(pl_type), params=params)


def player_info_resp(st_type, nhl_id):
    api_end = f'?hydrate=stats(splits={st_type})'
    url = ''.join([PL_STAT_URL_BASE, str(nhl_id), api_end])

    response = requests.get(url)
    response.raise_for_status()
    return response


def init_list(request, response, list_name):
    user = request.user
    favorites = user.favorite.all()
    for player in response:
        player['playerHeight'] = HEIGHT_CONVERT[player['playerHeight']]

        if player['playerPositionCode'] in POSITIONS[5:]:
            player['playerPositionCode'] += WING

        if player['playerDraftOverallPickNo'] is None:
            player['playerDraftOverallPickNo'] = '—'
            player['playerDraftYear'] = '—'

        if len(player['playerTeamsPlayedFor']) > 3:
            player['playerTeamsPlayedFor'] = (
                player['playerTeamsPlayedFor'].split(',')[-1]
            )

        for item in favorites:
            if player['playerName'] == item.name:
                player['is_favorite'] = True

        list_name.append(player)
    return list_name


def get_player(nhl_id):
    return get_object_or_404(Player, nhl_id=nhl_id)


def goalies_list(request):
    goalies = []
    goalies_summ = players_resp(REP_TYPE1, PL_TYPE1).json()['data']
    return init_list(request, goalies_summ, goalies)


def skaters_list(request):
    skaters = []

    summary = players_resp(REP_TYPE2, PL_TYPE2).json()['data']
    realtime = players_resp(REP_TYPE3, PL_TYPE2).json()['data']
    toi = players_resp(REP_TYPE4, PL_TYPE2).json()['data']
    init_list(request, summary, skaters)

    for count, player in enumerate(realtime):
        skaters[count]['hits'] = player['hits']
        skaters[count]['blockedShots'] = player['blockedShots']
        skaters[count]['faceoffsWon'] = player['faceoffsWon']

    for count, player in enumerate(toi):
        skaters[count]['timeOnIcePerGame'] = (
            time_from_sec(player['timeOnIcePerGame'])
        )
        skaters[count]['ppTimeOnIcePerGame'] = (
            time_from_sec(player['ppTimeOnIcePerGame'])
        )
        skaters[count]['shTimeOnIcePerGame'] = (
            time_from_sec(player['shTimeOnIcePerGame'])
        )

    return skaters


def gamelog(nhl_id):
    log = []
    response = player_info_resp(STAT_TYPE4, nhl_id)
    log_json = response.json()['people'][0]['stats'][0]['splits']
    for game in log_json:
        game['date'] = date_convert(game['date'])
        log.append(game)
    return log


def date_convert(date):
    datetime_obj = datetime.datetime.strptime(date, '%Y-%m-%d')
    return datetime_obj.strftime('%b %e')


def is_favorite(request, nhl_id):
    player = get_player(nhl_id)
    favor = False

    if player.favorite.filter(id=request.user.id).exists():
        favor = True
    return favor


def season_stats(nhl_id):
    response = player_info_resp(STAT_TYPE1, nhl_id)
    return response.json()['people'][0]


def career_stats(nhl_id):
    response = player_info_resp(STAT_TYPE3, nhl_id)
    return response.json()['people'][0]['stats'][0]['splits'][0]['stat']


def sbs_stats(nhl_id):
    seasons = []
    player = player_info_resp(STAT_TYPE2, nhl_id).json()
    seasons_json = player['people'][0]['stats'][0]['splits']
    position = player['people'][0]['primaryPosition']['code']
    for season in seasons_json:
        if season['league']['name'] == NHL:
            season['season'] = format_season(season['season'])
            # Getting an average TOI from total
            if position in POSITIONS[1:]:

                season['stat']['timeOnIce'] = (
                    time_from_sec(time_to_sec(season['stat']['timeOnIce'])
                    /season['stat']['games'])
                )

                season['stat']['powerPlayTimeOnIce'] = (
                    time_from_sec(time_to_sec(season['stat']['powerPlayTimeOnIce'])
                    /season['stat']['games'])
                )

                season['stat']['shortHandedTimeOnIce'] = (
                    time_from_sec(time_to_sec(season['stat']['shortHandedTimeOnIce'])
                    /season['stat']['games'])
                )

            seasons.append(season)
    return seasons


def format_season(season):
    return f'{season[:4]}-{season[6:]}'


def roster(request, slug, team_id):
    players = goalies_list(request) + skaters_list(request)

    team_roster = [player for player
                   in players
                   if player['playerTeamsPlayedFor'] == TEAM_ABBR[team_id]]
    return team_roster


def roster_position(request, rost, pos, slug, team_id):
    return [player for player
            in rost
            if player['playerPositionCode'] in pos]

Answer 1

At a high level you definitely have the right idea about pulling things out of your views. In MVC, we generally keep business logic out of views. That includes things like calculating player stats and information, so this definitely seems like a good choice.

I can fairly confidently say you correctly identify the actual speed issue (but you dismiss it). Speaking very generally, unless you're doing something super crazy, a couple of loops with a few hundred items aren't going to cause much of a problem. This can only truly be confirmed by profiling, but if there are pages that require API requests to 800 different URLs, then that's almost surely your bottleneck.

If you wanted to confirm this, you could place a proxy (that ignores Cache-Control/Expires as the API endpoints may not properly set it--see Squid) between the NHL site and your app. This will make those 800+ requests MUCH faster. Your site should speed up significantly. As a quick temporary solution, this is actually pretty decent as you'll likely get a decent speed boost for free. You'll probably want to configure Squid to cache responses for a few hours.

Another easier way to achieve this would be to decorate your services functions with a caching decorator. This isn't as good as the proxy, because unless it persists to disk, your cache won't survive rebooting the server, but this doesn't require learning how to setup Squid.

In the long run, I'd recommend running the functions in services periodically in a separate worker process which saves results to a DB. Then, in your webapp, query your own DB for the results you need. Why?

For one, the NHL site probably doesn't appreciate you sending 800-some requests (this is a guess from your numbers, I didn't read the code too carefully) EVERY time someone visits a page on your site. Depending on how much traffic you get, it could cause them to block or throttle you. They may even impose global rate limits. Part of being a good API consumer is caching things so you don't have to tax the API.

Secondly, your webserver can only handle so many TCP connections. IIRC requests does keepalive and connection pooling, but your web server runs across multiple processes these might not be shared. In any event, any outbound requests beyond those absolutely necessary (which, as I argue above, is none) will decrease your capacity to handle incoming traffic.

Third, if you want to do any analytics on your end that does take a non-negligible amount of time (say, some sort of fancy predictions using regressions or other models), doing it outside incoming web requests means your site won't become any slower if you choose to add something like this.