4
\$\begingroup\$

My first (real?) program.

The client connects to a Django restframework API by sending user credentials to obtain a token for headers. Then the user can drag and drop an image from a OS folder into the terminal and press enter, then write a title for it to upload.

The image(s) URL is obtained from the server and then turned into markdown that are listed in a bytearray. This byte array gets loaded into a tempfile that can be edited with the system's default editor or Vim. When this is saved and closed, it's converted to html and the article is published.

api_client.py

import requests
import tempfile
import os
import pypandoc
import json
from getpass import getpass
from subprocess import call

EDITOR = os.environ.get('EDITOR', 'vim')
ARTICLE_URL = 'http://127.0.0.1:8000/api/articles/'
IMAGE_URL = 'http://127.0.0.1:8000/api/media/images/'
TOKEN_URL = 'http://127.0.0.1:8000/api-auth-token/'


def obtain_token(username, password):
    """Authenticate and obtain token"""
    data = {'username': username, 'password': password}
    req = requests.post(TOKEN_URL, data=data)
    res = req.json()
    token = res['token']
    headers = {'Authorization': 'Token {}'.format(token)}
    return headers


"""
Ask for user credentials to set header, not sure how to 
place this code in a functional way
"""
username = input("Username: ")
password = getpass()

headers = obtain_token(username=username, password=password)


def image_upload(img_file, img_title):
    """Upload image with title"""
    files = {'image': img_file}
    payload = {'title': img_title}
    upload = requests.post(IMAGE_URL, files=files, data=payload,
                           headers=headers)
    return upload


md_urls = bytearray()


def img_docload():
    """
    Get the latest uploaded image and convert it
    to a html string so that pypandoc again can make
    it into markdown, then extend each to the bytearray.
    """
    get_url = requests.get(IMAGE_URL)
    get_json = json.loads(get_url.content)
    clean = get_json[-1]['image']
    md_html = "<img src='"+clean+"'>"
    md = pypandoc.convert_text(md_html, 'md', format='html')
    md_urls.extend(md.encode())


def article(headline, summary):
    """
    Make a tempfile and write the list of markdown inserts,
    then open it in Vim or any default editor. Save and quit
    to convert from markdown to html and upload the article.
    """
    with tempfile.NamedTemporaryFile(suffix='.md') as tmp:
        tmp.write(md_urls)
        tmp.flush()
        call([EDITOR, tmp.name])
        tmp.seek(0)
        edited = tmp.read()
        article = edited.decode('utf-8')
        content = pypandoc.convert_text(article, 'html', format='md')
        payload = {
            'headline': headline,
            'summary': summary,
            'content': content,
            }
        upload = requests.post(ARTICLE_URL, json=payload, headers=headers)
        return upload


def main():
    while True:
        action = input("Upload image? (k): ")
        if action == 'k':
            img_file = open(input("Image - Filename: ").strip('\''), 'rb')
            img_title = input("Image - Title: ")
            image_upload(img_file=img_file, img_title=img_title)
            img_docload()
            continue
        else:
            headline = input("Article - Headline: ")
            summary = input("Article - Summary: ")
            article(headline=headline, summary=summary)
            print("Article is published")
            break

main()

Questions:

  • I wrote the program without a single function to begin with. Then I tried to follow the functional design to learn more about functions and how to structure programs. What could be done better?
  • How would you place the obtain_token function and its I/O?
  • My next goal is to learn about classes and OOP; is there a place for classes in this program or is it unnecessary?
  • What do you think about the way it first upload an image and then gets the URL, then puts it into a HTML img element, and then converts it to md (pypandoc needs it to make the markdown, and I wanted the links to be "dynamic")? This can't be the best approach; would you do it server-side? Or is it best to do as much as possible on the client-side?

In general I would like to know about all the mistakes I have made - what could be done better - especially regarding structure, before I move on to learn more.

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$
ARTICLE_URL = 'http://127.0.0.1:8000/api/articles/'
IMAGE_URL = 'http://127.0.0.1:8000/api/media/images/'
TOKEN_URL = 'http://127.0.0.1:8000/api-auth-token/'

The repetition here could be harmful. If you move the server, then there's three places that need to be edited and kept in sync. It's easier to cope with such a move by reducing the duplication:

BASE_URL = 'http://127.0.0.1:8000/api/'
ARTICLE_URL = BASE_URL + 'articles/'
IMAGE_URL = BASE_URL + 'media/images/'
TOKEN_URL = BASE_URL + 'api-auth-token/'

It's good to see the with keyword used effectively to ensure that resources are cleaned up properly, even if exceptions are thrown; keep up the good habits! ;-)


In main(), the while True is misleading; the actual flow is that we repeat the first branch a number of times, and then the second branch exactly once. We can restructure the loop to make that clearer:

def main():
    while input("Upload image? (k): ") == 'k':
        img_file = open(input("Image - Filename: ").strip('\''), 'rb')
        img_title = input("Image - Title: ")
        image_upload(img_file=img_file, img_title=img_title)
        img_docload()

    headline = input("Article - Headline: ")
    summary = input("Article - Summary: ")
    article(headline=headline, summary=summary)
    print("Article is published")

Without continue and break it's much easier to understand the flow of control; use these keywords only as a last resort!


We should run main() only if we're directly executing this as a program, not importing it as a module:

if __name__ == '__main__':
    main()

We also need to move the username/password request into the main(), and we probably want to pass headers around as parameter rather than sharing a global variable. (I assume that passing passwords around in plaintext is a weakness that is forced on you by the API you're using - when you're designing interfaces, you must never pass authenticators insecurely like that!).

\$\endgroup\$

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.