0
\$\begingroup\$

I wrote the following sample script for learning purposes:

import requests
import json
import pyodbc
import os
import sys

source_url = 'https://jsonplaceholder.typicode.com/posts/'

query_posts_response = requests.get(source_url)

if (query_posts_response.status_code == 200):
    posts = json.loads(query_posts_response.content.decode('utf-8'))

db_connection = pyodbc.connect("Driver={SQL Server Native Client 11.0};"
    "Server=(local);"
    "Database=sandbox;"
    "Trusted_Connection=yes;")

sys_devnull = open(os.devnull, 'w')
sys_stdout_original = sys.stdout
sys.stdout = sys_devnull

db_cursor = db_connection.cursor()

for post in posts:
    db_cursor.execute("INSERT INTO dbo.Posts(id, title) VALUES (?, ?)", post['id'], post['title'])

db_connection.commit()
db_connection.close()

sys.stdout = sys_stdout_original

I'm grabbing content from a REST API, and I'm inserting it into a relational table.

I write scripts in other languages, but this is my first Python script. I don't know what I don't know. I'm looking for a critique. I realize I was lazy on the database call and didn't create an exception handler. That said, is this the generally accepted way to interface with a REST endpoint and interact with a database?

Concerning output of db_cursor.execute(), see the following:

enter image description here

\$\endgroup\$
6
  • 2
    \$\begingroup\$ Why do you redefine sys.stdout during the database operations? \$\endgroup\$ Oct 24, 2018 at 21:34
  • 1
    \$\begingroup\$ I was running this through the REPL while I was writing it. The execute() call generated all this jibber jabber so I piped it to /dev/null. Probably doesn't make sense in a script? \$\endgroup\$
    – Adam
    Oct 24, 2018 at 21:36
  • \$\begingroup\$ execute() should be silent. It doesn't ever make sense to squelch stdout. \$\endgroup\$ Oct 24, 2018 at 21:37
  • \$\begingroup\$ Noted feedback on not squelching stdout as a best practice; thank you. Sadly, execute() is not silent - see revised post. \$\endgroup\$
    – Adam
    Oct 25, 2018 at 18:54
  • \$\begingroup\$ You only see those results because you are running the code in a REPL. That's the point of the "P" in "REPL". If you don't want to see the results, don't run the code in a REPL. \$\endgroup\$ Oct 25, 2018 at 19:52

1 Answer 1

1
\$\begingroup\$

Some suggestions:

  1. A linter (or two) such as pycodestyle or flake8 will help you write more idiomatic Python. For example, brackets are not required around an if statement.
  2. You can use query_posts_response.json() to easily get JSON response content.
  3. I can't see a reason why this script should do anything with the input and output streams.
  4. db_connection.close() should be in a finally block. The try block should start with creating the connection.
  5. If you ever intend to reuse the code it should be wrapped inside at least a main function (which orchestrates argument parsing, handling exceptions which should lead to non-zero return codes, and the like) and a separate function to do the work.
\$\endgroup\$
1
  • \$\begingroup\$ This is exactly the sort of feedback I was looking for. \$\endgroup\$
    – Adam
    Oct 25, 2018 at 18:57

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.