Simple command line API client for getting quotes

Question

Was given a take-home test as part of a job interview, which I failed due to a supposed lack of tests, logging, error handling and packaging. But I did touch upon all of these.. suggested time spent was only 2 hours. The task was to create a command line tool in Python that integrates with a trading API.

Not the most experienced developer here, so I'd like to know what gave this away.

The API to integrate with had a /quote endpoint to get price quotes for financial instruments, an /order endpoint to make a trade, an /instruments endpoint to get the list of available financial instruments and a /balance endpoint to get current account balance (in the different instruments).

Project structure

my_project/
 logs/
 tests/
 __init__.py
 client.py
 main.py
 README.md
 requirements.txt
 services.py
 settings.py
 utils.py

main.py

from my_project import services
from my_project.client import MyProjectClient
from my_project.settings import parser
my_project_client = MyProjectClient()
args = parser.parse_args()
services.trade(my_project_client, **vars(args))

client.py

import datetime
import logging
import time
from typing import Optional
from uuid import uuid4
import requests
from my_project import settings
class AbstractAPIClient:
 def __init__(self):
 self.api_url: str = NotImplemented
 self.token: str = NotImplemented
 def call_api_endpoint(
 self, *, method, path, data=None
 ) -> Optional[requests.Response]:
 response = None
 try:
 response = requests.request(
 method,
 f"{self.api_url}/{path}/",
 headers={"Authorization": f"Token {self.token}"},
 json=data,
 )
 response.raise_for_status()
 logging.info(f"{response} {response.content}")
 except requests.exceptions.HTTPError as err:
 logging.error(f"{err} {err.response.text}")
 return response
class MyProjectClient(AbstractAPIClient):
 def __init__(self):
 super().__init__()
 self.api_url = settings.MY_PROJECT_API_URL
 self.token = settings.MY_PROJECT_API_TOKEN
 def quote(self, *, side, quantity, instrument) -> Optional[requests.Response]:
 uuid = uuid4()
 post_data = {
 "side": side,
 "quantity": quantity,
 "instrument": instrument,
 "client_rfq_id": str(uuid),
 }
 return self.call_api_endpoint(
 method="POST", path=settings.MY_PROJECT_QUOTE_PATH, data=post_data
 )
 def order(
 self, *, side, quantity, instrument, price
 ) -> Optional[requests.Response]:
 uuid = uuid4()
 valid_until = datetime.datetime.utcfromtimestamp(time.time() + 10)
 post_data = {
 "side": side,
 "quantity": quantity,
 "instrument": instrument,
 "client_order_id": str(uuid),
 "price": price,
 "type": "gck",
 "valid_until": valid_until.strftime("%Y-%m-%dT%H:%M:%S"),
 "execution_strategy": "risk-adding",
 }
 return self.call_api_endpoint(
 method="POST", path=settings.MY_PROJECT_ORDER_PATH, data=post_data
 )
 def balance(self) -> Optional[requests.Response]:
 return self.call_api_endpoint(method="GET", path=settings.MY_PROJECT_BALANCE_PATH)
 def instruments(self) -> Optional[requests.Response]:
 return self.call_api_endpoint(method="GET", path=settings.MY_PROJECT_INSTRUMENTS_PATH)

services.py

import json
from decimal import Decimal
from my_project.client import MyProjectClient
from my_project.settings import TradeInstrument, TradeSide
from my_project.utils import print_response, yes
def trade(
 api_client: MyProjectClient, side: TradeSide, quantity: int, instrument: TradeInstrument
) -> None:
 order_response = None
 quote_response = api_client.quote(
 side=side, quantity=quantity, instrument=instrument
 )
 json_quote_response = json.loads(quote_response.content)
 print_response(quote_response)
 if quote_response:
 side = json_quote_response["side"]
 quantity = json_quote_response["quantity"]
 instrument = json_quote_response["instrument"]
 price = json_quote_response["price"]
 expected_change_in_balance = get_expected_change_in_balance(
 side=side,
 quantity=Decimal(quantity),
 instrument=instrument,
 price=Decimal(price),
 )
 print(
 "Expected change in balances:\n",
 json.dumps(expected_change_in_balance, indent=4, default=str),
 )
 if yes("Execute trade?"):
 order_response = api_client.order(
 side=side, quantity=quantity, instrument=instrument, price=price
 )
 print_response(order_response)
 if order_response:
 input("Press Enter to continue. ")
 balance_response = api_client.balance()
 print_response(balance_response)
def get_expected_change_in_balance(
 *, side: TradeSide, quantity: Decimal, instrument: TradeInstrument, price: Decimal
):
 base_currency_sign = 1
 if side == TradeSide.SELL:
 base_currency_sign = -1
 return {
 "GBP": base_currency_sign * quantity,
 instrument[3:6]: -1 * base_currency_sign * price * quantity,
 }

settings.py

import argparse
import logging
# Environment variables
MY_PROJECT_API_URL = "https://api.uat.my_project.net"
MY_PROJECT_API_TOKEN = "02edad94e19ecb63c690a719d38c898616899d10"
MY_PROJECT_QUOTE_PATH = "request_for_quote"
MY_PROJECT_INSTRUMENTS_PATH = "instruments"
MY_PROJECT_BALANCE_PATH = "balance"
MY_PROJECT_ORDER_PATH = "order"
# Logging
logging.basicConfig(
 filename="logs/my_project.log",
 level=logging.INFO,
 format="%(asctime)s %(levelname)-8s %(message)s",
 datefmt="%Y-%m-%d %H:%M:%S",
)
# Constants
class TradeSide:
 BUY = "buy"
 SELL = "sell"
class TradeInstrument:
 GBPUSD_SPOT = "GBPUSD.SPOT"
 GBPJPY_SPOT = "GBPJPY.SPOT"
TRADE_SIDE_CHOICES = (
 TradeSide.BUY,
 TradeSide.SELL,
)
TRADE_INSTRUMENT_CHOICES = (
 TradeInstrument.GBPUSD_SPOT,
 TradeInstrument.GBPJPY_SPOT,
)
# CLI argument parser
parser = argparse.ArgumentParser(description="Trade with MyProject")
parser.add_argument("side", choices=TRADE_SIDE_CHOICES)
parser.add_argument("quantity", type=float)
parser.add_argument("instrument", choices=TRADE_INSTRUMENT_CHOICES)

utils.py

import json
from typing import Optional
import requests
def yes(question):
 prompt = f"{question} (y/n) "
 ans = input(prompt).strip().lower()
 return ans and ans[0] == "y"
def print_response(response: Optional[requests.Response]) -> None:
 if response is None:
 print("No response was received.")
 else:
 json_response = json.loads(response.content)
 print(json.dumps(json_response, indent=4))

tests/test_client.py

from my_project.client import MyProjectClient
from my_project.settings import TradeInstrument, TradeSide
def test_quote(mocker):
 mocked_call_api_endpoint = mocker.patch(
 "my_project.client.AbstractAPIClient.call_api_endpoint"
 )
 client = MyProjectClient()
 client.quote(
 side=TradeSide.BUY, quantity=10, instrument=TradeInstrument.GBPJPY_SPOT
 )
 mocked_call_api_endpoint.assert_called_once()

setup.py

I haven't done packaging before, so I just put this setup.py file one directory above the project directory:

from setuptools import find_packages, setup
setup(name='my_project', version='1.0', packages=find_packages())

Answer 1

One way to improve the structure would be to modularize your code. Once your client is a "real" module, you can convert your main.py into a __main__.py which offers some advantages when it comes to execution convenience.

Why have an AbstractAPIClient with only one implementation? This seems like premature abstraction. Even if you did need it,

 self.api_url: str = NotImplemented
 self.token: str = NotImplemented

is sort of a strange implementation. If you wanted "abstract properties", you would instead write

@property
def api_url(self) -> str:
 raise NotImplementedError()

Or easier yet, simply have those properties be concrete instead of abstract, and accepted as constructor parameters on the superclass.

These parameters:

 method, path, data=None

could use type hints, at a guess str, str and Optional[Dict[str, Any]].

Your call_api_endpoint should be using with on the response. This probably won't have any effect if you're not in streamed mode, but it's safer to do it than not.

call_api_endpoint also converts a useful error interface - an exception - into a less-useful error interface - an in-band None-or-not response. The more structured thing to do is either let the original exception fall through, or wrap it in your own exception type and use a proper raise .. from.

time.time will produce some nasty surprises during edgy times surrounding daylight savings switch for instance; you need to use monotonic instead.

10 seems like a lifetime constant that should be factored out, perhaps into your settings.

Your post_data dict is an opportunity to factor out a well-typed class that has a serialization routine, to separate the data concern from the API communication concern.

The methods in your client accept domain data (e.g. price) as parameters, but do not likewise deserialize the response into domain data - instead they return the raw response. This is asymmetric. Accepting one domain data class and returning another would be more symmetric. trade likewise suffers from a scramble in responsibility - it deals with domain data, e.g. quantity, but also has to care about JSON where it should not.

TradeSide and TradeInstrument are perfect examples that should be made Enums - they're very close already! Among other advantages, TRADE_SIDE_CHOICES and TRADE_INSTRUMENT_CHOICES can go away since enumeration classes are themselves iterable over their values.

settings.py has a collection of things that don't belong. You have

• domain data classes (Trade*) that should belong in a business module;
• actual settings constants MY_PROJECT_* that should belong in a configuration file and not a .py, to make possible a Unix filesystem-level security where the code can be globally read-only and sensitive credentials can only be readable for a restricted user group; and
• a parser that belongs in a __main__.py.

Your yes method could stand to have a better "action" name such as ask_yes_no; parameter and return type hints; and a validation loop. Also,

ans and ans[0] == "y"

is better-represented as

ans.startswith('y')

which will deal with the empty-string case just fine.

This message:

print("No response was received.")

is a lie. If there's a failure that you're swallowing and converting into None, then you will have received a response - it's just a failure response instead of a success response. There are other issues with print_response: it's a debugging routine to dumps some JSON; but this debugging routine is unconditionally baked into your main logic. If you wanted to keep this, you'd want to logging.debug it instead of just printing it; that way the output can be suppressed under normal circumstances.