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\$
  • 2
    \$\begingroup\$ Why do you redefine sys.stdout during the database operations? \$\endgroup\$ – 200_success Oct 24 '18 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 '18 at 21:36
  • \$\begingroup\$ execute() should be silent. It doesn't ever make sense to squelch stdout. \$\endgroup\$ – 200_success Oct 24 '18 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 '18 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\$ – 200_success Oct 25 '18 at 19:52
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.
|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ This is exactly the sort of feedback I was looking for. \$\endgroup\$ – Adam Oct 25 '18 at 18:57

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.