HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

pyvdp - Python library for Visa Developer Program

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
visapyvdpprogramforpythonlibrarydeveloper

Problem

I'm coming from PHP and recently started developing in Python. As a part of my self-study course, I developed a small library for access to Visa Developer Program APIs and published it on Github. It is bundled with demo Django app. Here I'm sharing a module that implements an abstract connection to VDP.

I'd appreciate hearing your comments regarding the source code. Personally, I have a feeling that modules hierarchy could have been organized differently and maybe I'm overusing OOP approaches.

visa/request.py

``
import jsonpickle
import os
import requests

try:
import urllib.parse as urllib_parse
except ImportError:
# Python 2.7 compatibility
from urllib import parse as urllib_parse

try:
import configparser as parser
except ImportError:
import ConfigParser as parser

from visa.exceptions import (VisaAccessDeniedError, VisaDuplicateTransactionError, VisaGeneralError,
VisaMessageValidationError, VisaNotFoundError, VisaTimeoutError, VisaUnauthenticatedError)

class VisaRequest:
"""
Constructs a request to Visa API using provided transaction, resource, method and http verb.
Request can be submitted with request() method, which returns a dictionary in following format:
{'code': http_response_code, 'message': json-encoded content}
This class is not assumed to be instantiated on its own (consider it abstract). Instead, every VISA API
implementation must provide its own request class, inheriting from VisaRequest and providing a corresponding VISA
API resource name as a
resource` argument value.
"""
config = parser.ConfigParser()
config_file = os.path.join(os.path.dirname(__file__), 'configuration.ini')
config.read(config_file)

def __init__(self, resource, api, method, http_verb):
"""
:param resource: VISA API resource name
:type resource: str
:param api: VISA API api name
:type api: str
:param method: VISA API endpoint method

Solution

The code overall looks clean, there will always be debates on writing classes or not (both approaches have great reasoning behind them, of course):

  • Stop Writing Classes (PyCon 2012)



  • Start Writing More Classes (2013)



Some things you can improve:

-
the status code handling. Build a mapping between status codes and exception types - this would help to make the code more modular, configurable and concise, something like:

ERROR_CODES = {
    202: VisaTimeoutError,
    303: VisaDuplicateTransactionError,
    400: VisaMessageValidationError,
    401: VisaUnauthenticatedError,
    403: VisaAccessDeniedError,
    404: VisaNotFoundError
}
if code == 200:
    return result
else:
    raise ERROR_CODES.get(code, VisaGeneralError)(result=result)


-
you can avoid some nestedness in the __to_vd() method:

if not transaction:
    return

return jsonpickle.encode(transaction, unpicklable=False)


-
since you are repeatedly getting the current directory name and, it is not going to change, do it once beforehand and reuse:

CUR_DIR_NAME = os.path.dirname(__file__)


-
I would also initialize a session via requests.Session() in the class constructor, define the session.headers, session.auth and other required parameters. Then, when you will make API requests using self.session, all of the parameters you've initially set will be automatically applied. This will also have a positive performance impact:


..if you're making several requests to the same host, the underlying TCP
connection will be reused, which can result in a significant
performance increase

-
follow the PEP8 import organization guidelines

Code Snippets

ERROR_CODES = {
    202: VisaTimeoutError,
    303: VisaDuplicateTransactionError,
    400: VisaMessageValidationError,
    401: VisaUnauthenticatedError,
    403: VisaAccessDeniedError,
    404: VisaNotFoundError
}
if code == 200:
    return result
else:
    raise ERROR_CODES.get(code, VisaGeneralError)(result=result)
if not transaction:
    return

return jsonpickle.encode(transaction, unpicklable=False)
CUR_DIR_NAME = os.path.dirname(__file__)

Context

StackExchange Code Review Q#155350, answer score: 4

Revisions (0)

No revisions yet.