2
\$\begingroup\$

I have a simple piece of code which connects to an API, does a request and gets back the raw data. After getting this data I create several objects (models) and persist them into a MongoDB database.

Example of a model:

class Match:
    def __init__(self, data):
        self.data = data

    @property
    def match_id(self):
        """The Match id"""
        return self.data['attributes']['id']
    ...

Example of API CLient

class Client:
    def __init__(self):
        self.session = requests.Session()

    def request(self, endpoint, params=None):
        response = self.session.get(endpoint, timeout=TIMEOUT, params=params)
        res_code = response.status_code
        if res_code != API_OK:
            exception = API_ERRORS.get(res_code)
            raise exception
        return response.json()

Example of the controller:

class Handler(Client):
    def __init__(self, api_key):
        super().__init__()
        self.api_key = api_key

    def query_match(self, match_id, shard='default'):
        res = []
        url = f'{SHARD_URL}/{shard}/matches/{match_id}'
        res = self.request(url)
        return Match(res)

And then I have the logic for getting the data into mongoDB after calling get_player

class BuildAndStore(Handler):
    def __init__(self, mongo_client, api_key):
        super().__init__(api_key)
        self.mongo_client = mongo_client
        self.players = list_players

    def process_matches(self, list_matches):
        ret = [] 
        for m_i in list_matches:
            match = self.query_match(m_id)
            json_data = build_json(match) # Extracts certain class properties
            ret.append(json_data)
        mongo.insert_many('matches', ret)

Having in mind I want to start consulting the API in parallel (multiple threads or moving to async) my questions are:

  • Is this pythonic? Is there a better way to do so?
  • Currently the Handler does the query and the Model receives the payback data. Instead of proceeding passing the JSON data, should I do the query for each model within their own class, passing the match_id and inheriting from "Handler" class Match(Handler): ?
\$\endgroup\$
0
\$\begingroup\$

In the request method the exception variable really doesn't have to be set before using it, just inline it and make the code more succinct:

def request(self, endpoint, params=None):
    response = self.session.get(endpoint, timeout=TIMEOUT, params=params)
    res_code = response.status_code
    if res_code != API_OK:
        raise API_ERRORS[res_code]
    return response.json()

Also, I'm guessing API_ERRORS[res_code] will also work and be more expected for any reader.

In the query_match method res doesn't have to be initialised, that's not pythonic actually:

def query_match(self, match_id, shard='default'):
    url = f'{SHARD_URL}/{shard}/matches/{match_id}'
    return Match(self.request(url))

The process_matches method could be a bit simpler:

def process_matches(self, list_matches):
    ret = []
    for m_i in list_matches:
        ret.append(build_json(self.query_match(m_id)))
    mongo.insert_many('matches', ret)

Or, there's the option to go for a less imperative list comprehension too:

def process_matches(self, list_matches):
    ret = [build_json(self.query_match(m_id))
           for m_i in list_matches]] 
    mongo.insert_many('matches', ret)

Regarding your second question, if I read it right, no, not a good idea from what I can tell from the code, leaving Match as a pure data object will serve you better in the long run. It's better if the two responsibilities aren't mixed. It'll also make it more clear what's being tested in your test cases if you can clearly distinguish between something that just holds the data and a API client (wrapper) that does networking (for example).

\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks for the answer, the refactoring proposed makes total sense to me. For the second topic, I think Match is already a data object as it receives a raw JSON, it extracts the data which later will be used as property and all the logic/requests are being done by Handler, am I right? \$\endgroup\$ – Koehler Aug 22 at 21:20
  • \$\begingroup\$ Yes, that's what I meant, exactly. \$\endgroup\$ – ferada Aug 22 at 22:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.