2
\$\begingroup\$

I am writing a python REST API library for Starburst and am trying to figure the best way to structure the code so it's dry but also easy to understand. Below is an example of code that is not dry.

Other methods I've researched is having a config file with the different API calls and having a single function for get/put/delete/post.

Other way is grouping based on domains and passing the method as a variable.

class StarburstAPI(object):

    def __init__(self, username=None, password=None, oauth=None, session=None):

        self.api_version = 'v1'
        self.base_url = 'https://starburst.com/api/' + self.api_version

        self.headers = default_headers()
        self.params = {}

        if session is None:
            session = requests.Session()
        self._session = session

        if username is not None or password is not None:
            self._session.auth = (username, password)

        self._session.cookies = self._session.head(self.base_url).cookies
        self._session.headers.update({'Content-Type': 'application/json'})

    
    # Get list of domains
    def list_domains(self):

        url = self.base_url + '/dataProduct/domains/'

        return self._session.get(url)
    

    # Get domain by id
    def get_domain(self, id: str):

        url = self.base_url + '/dataProduct/domains/' + id

        return self._session.get(url)


    # Create domain
    def create_domain(self, data=None, **kw):

        url = self.base_url + '/dataProduct/domains/'

        if data:

            kw = add_json_headers(kw)
            data = json.dumps(data)
            
        return self._session.post(url, data, **kw)


    # Delete domain by id
    def delete_domain(self, id: str):

        url = self.base_url + '/dataProduct/domains/' + id

        return self._session.delete(url)
    

    # Update domain by id
    def update_domain(self, id: str, data=None, **kw):

        url = self.base_url + '/dataProduct/domains/' + id

        if data:

            kw = add_json_headers(kw)
            data = json.dumps(data)

        return self._session.put(url, data, **kw)
    
\$\endgroup\$
2
  • 2
    \$\begingroup\$ "trying to figure the best way to structure the code so it's DRY but also easy to understand" -- if you are considering several competing approaches to organizing this code, there's still time to edit the Question, mentioning their pros and cons. Once a reviewer offers an Answer then edits will be frozen. \$\endgroup\$
    – J_H
    May 2, 2023 at 19:42
  • \$\begingroup\$ Is this Python 2 or 3? You use Python 2 syntax in your class declaration and the rest is ambiguous. \$\endgroup\$
    – Reinderien
    Jun 4, 2023 at 2:11

1 Answer 1

1
\$\begingroup\$
        self.api_version = 'v1'

No need to create lots of object attributes, when a local variable would do. (The variable name is helpful, thank you.)

       api_version = 'v1'

Here's another nit.

        if session is None:
            session = requests.Session()
        self._session = session

This is clear enough. We could express it more compactly with this common idiom:

        self._session = session or requests.Session()

Similarly you could write this slightly more compact expression:

        if username or password:
            self._session.auth = (username, password)

It's not strictly identical, but a username of "" empty string likely is of little interest.


You could delete this comment, for example:

    # Get list of domains
    def list_domains(self):

Avoid # comments that don't tell us something beyond what the code already said.

We put the "how", the technical details, into the code. We put the "why" in the comments or method docstring.


http verb

I really like how {list,create}_domain are distinct methods. Keep that organization.


repeated URLs and URL parents

You have several methods that assign url as this plus that. It seems to trouble you. I wouldn't worry about it too much. As written the code is very clear, and it's easy for a maintainer to search for matching URLs.

You might remove the url temp var, preferring to just hand a catenation expression in to .get or .post.

Consider defining a manifest constant of DOMAINS = '/dataProduct/domains/'. Or defining a trivial helper for that popular prefix.

This client library sometimes catenates '/dataProduct/domains/' + id. On the server side, a flask library would spell it this way: '/dataProduct/domains/{id}'. Consider adopting such a convention, and parsing out the "variable" portion of each URL. Remember that inspect is available to you. If a bunch of methods really are identical except for the URL, it should be possible to define them as one-liners, given appropriate support routines.

A helper for a common parent, such as DOMAINS, may assist in concisely defining accessors for several URLs immediately beneath it.

Your .post / .put methods have common kw + data needs, which could be extracted to a common helper.


Overall? Sure, this code is repetitive. But it's not that bad.

I am reminded of unit test code, where copy-n-paste (the enemy of DRY!) is explicitly accepted as a good practice. Why? To produce simple code that is easily understood and maintained.

Don't sell yourself short on those "verbose" URL strings that seem to trouble you. One of the first things a maintainer chasing a bug will need to do is grep for a URL or a fragment of one. The OP expresses URLs in very simple form, and that makes the codebase easily grep'able.

This code achieves many of its design goals.

I would be willing to delegate or accept maintenance tasks on it.


EDIT

Suppose you have a function that computes something useful, such as the root of an equation. And it also wants to tailor its action according to who called it. It's not hard to learn the name of the caller.

import inspect

def sqrt_and_who_called_me(n):
    """Returns a root, and the name of the calling function."""
    frame = inspect.currentframe()
    return math.sqrt(n), frame.f_back.f_code.co_name


def fn():
    root, name = sqrt_and_who_called_me(2.0)
    assert name == "fn"

Instead of fn we might have

  • def domains():
  • def domains_id():
  • def domains_a():
  • def domains_b():

and a helper wants to know that we're in the domains hierarchy, or wants to know whether there's a suffix and that it's one of {id, a, b}.

The caller could divulge such naming details as a parameter passed into the helper, but that might not be very DRY. (For example namedtuple suffers from the whole Bond = namedtuple('Bond', ...) James Bond syndrome.) Choosing structured names for several GET / POST routines could let the helper tailor its behavior for each one while minimizing repetition. And of course @decorators are another tool in your toolbox.

Examining frame.f_back.f_back lets you navigate up the stack to the caller's caller, and so on.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks so much for your input! \$\endgroup\$
    – Ann Nguyen
    May 4, 2023 at 21:02
  • \$\begingroup\$ Regarding using inspect, can you give an example? I am not too familiar with using inspect. \$\endgroup\$
    – Ann Nguyen
    May 4, 2023 at 22:59
  • \$\begingroup\$ A call to .currentframe() can provide some useful context. \$\endgroup\$
    – J_H
    May 5, 2023 at 0:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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