0
\$\begingroup\$

The code base is split between three different files odbc.py, scrape.py, and dataprocessor.py. For ODBC, the job of this file is to take scraped data and determine whether or not those results are in a database. Ultimately, I'd like to extend this script to contain more general functionality, so that the queries that are being performed are less dependent on the specifics of checking for titles; for instance, I have an idea where I'd like to check what interviews were added to the database and return back the results and sample path. The 'top 100' variable is an example of the output from scrape.py.

Here is the code for pyodc.py:

import pyodbc
from ConfigFunc import GetConfig

#This function allows for the user to pull ODBC #
Location = 'AzureWinMedia06232019'
connectConfig = GetConfig(Location)

#Establishes Connection with Azure Database via ODBC#
conn = pyodbc.connect("""Driver={};Server={};Database={};Uid={};
Pwd={};Encrypt={};TrustServerCertificate={};
Connection Timeout={};""".format(connectConfig["Driver"], connectConfig["Server"], connectConfig["Database"], 
connectConfig["User"], connectConfig["Password"], connectConfig["Encrypt"], connectConfig["TrustedServer"], connectConfig["ConnectionTimeout"]))

top100 = {1: {'artist': 'Lil Nas X Featuring Billy Ray Cyrus', 'title': 'Old Town Road'}, 2: {'artist': 'Billie Eilish', 'title': 'Bad Guy'}, 3: {'artist': 'Khalid', 'title': 'Talk'}, 4: {'artist': 'Jonas Brothers', 'title': 'Sucker'}, 5: {'artist': 'Ed Sheeran & Justin Bieber', 'title': "I Don't Care"}, 6: {'artist': 'Post Malone', 'title': 'Wow.'}, 7: {'artist': 'Post Malone & Swae Lee', 'title': 'Sunflower (Spider-Man: Into The Spider-Verse)'}, 8: {'artist': 'DaBaby', 'title': 'Suge'}, 9: {'artist': 'Chris Brown Featuring Drake', 'title': 'No Guidance'}, 10: {'artist': 'Sam Smith & Normani', 'title': 'Dancing With A Stranger'}, 11: {'artist': 'Polo G Featuring Lil Tjay', 'title': 'Pop Out'}, 12: {'artist': 'Shawn Mendes', 'title': "If I Can't Have You"}, 13: {'artist': 'Ava Max', 'title': 'Sweet But Psycho'}, 14: {'artist': 'Taylor Swift Featuring Brendon Urie', 'title': 'ME!'}, 15: {'artist': 'Halsey', 'title': 'Without Me'}, 16: {'artist': 'Ariana Grande', 'title': '7 Rings'}, 17: {'artist': 'Lizzo', 'title': 'Truth Hurts'}, 18: {'artist': 'Marshmello & Bastille', 'title': 'Happier'}, 19: {'artist': 'Blake Shelton', 'title': "God's Country"}, 20: {'artist': 'Morgan Wallen', 'title': 'Whiskey Glasses'}, 21: {'artist': 'Panic! At The Disco', 'title': 'High Hopes'}, 22: {'artist': 'Luke Combs', 'title': 'Beer Never Broke My Heart'}, 23: {'artist': 'Daddy Yankee & Katy Perry Featuring Snow', 'title': 'Con Calma'}, 24: {'artist': 'Young Thug, J. Cole & Travis Scott', 'title': 'The London'}, 25: {'artist': 'J. Cole', 'title': 'Middle Child'}, 26: {'artist': 'City Girls', 'title': 'Act Up'}, 27: {'artist': 'benny blanco, Halsey & Khalid', 'title': 'Eastside'}, 28: {'artist': 'Katy Perry', 'title': 'Never Really Over'}, 29: {'artist': 'Mustard & Migos', 'title': 'Pure Water'}, 30: {'artist': 'Tyler, The Creator', 'title': 'Earfquake'}, 31: {'artist': 'Panic! At The Disco', 'title': 'Hey Look Ma, I Made It'}, 32: {'artist': 'Meek Mill Featuring Drake', 'title': 'Going Bad'}, 33: {'artist': 'Dan + Shay', 'title': 'Speechless'}, 34: {'artist': 'Lady Gaga & Bradley Cooper', 'title': 'Shallow'}, 35: {'artist': 'Khalid', 'title': 'Better'}, 36: {'artist': 'Lee Brice', 'title': 'Rumor'}, 37: {'artist': 'Ariana Grande', 'title': "Break Up With Your Girlfriend, I'm Bored"}, 38: {'artist': 'Travis Scott', 'title': 'Sicko Mode'}, 39: {'artist': 'Thomas Rhett', 'title': 'Look What God Gave Her'}, 40: {'artist': 'A Boogie Wit da Hoodie', 'title': 'Look Back At It'}, 41: {'artist': 'Calboy', 'title': 'Envy Me'}, 42: {'artist': 'Billie Eilish', 'title': "When The Party's Over"}, 43: {'artist': 'Halsey', 'title': 'Nightmare'}, 44: {'artist': 'Jonas Brothers', 'title': 'Cool'}, 45: {'artist': 'Luke Combs', 'title': 'Beautiful Crazy'}, 46: {'artist': 'Kane Brown', 'title': 'Good As You'}, 47: {'artist': 'Cardi B', 'title': 'Press'}, 48: {'artist': 'Lil Baby', 'title': 'Close Friends'}, 49: {'artist': 'Ed Sheeran Featuring Chance The Rapper & PnB Rock', 'title': 'Cross Me'}, 50: {'artist': 'YG, Tyga & Jon Z', 'title': 'Go Loko'}, 51: {'artist': 'Cardi B & Bruno Mars', 'title': 'Please Me'}, 52: {'artist': 'Brett Eldredge', 'title': 'Love Someone'}, 53: {'artist': 'Offset Featuring Cardi B', 'title': 'Clout'}, 54: {'artist': 'YK Osiris', 'title': 'Worth It'}, 55: {'artist': 'Lewis Capaldi', 'title': 'Someone You Loved'}, 56: {'artist': 'Kelsea Ballerini', 'title': 'Miss Me More'}, 57: {'artist': 'P!nk', 'title': 'Walk Me Home'}, 58: {'artist': 'Billie Eilish', 'title': 'Bury A Friend'}, 59: {'artist': 'Maren Morris', 'title': 'GIRL'}, 60: {'artist': 'DJ Khaled Featuring SZA', 'title': 'Just Us'}, 61: {'artist': 'Luke Bryan', 'title': "Knockin' Boots"}, 62: {'artist': 'Luke Combs', 'title': "Even Though I'm Leaving"}, 63: {'artist': '5 Seconds Of Summer', 'title': 'Easier'}, 64: {'artist': 'Summer Walker X Drake', 'title': 'Girls Need Love'}, 65: {'artist': 'Lil Tecca', 'title': 'Ran$om'}, 66: {'artist': 'Blanco Brown', 'title': 'The Git Up'}, 67: {'artist': 'Meek Mill Featuring Ella Mai', 'title': '24/7'}, 68: {'artist': 'Jason Aldean', 'title': 'Rearview Town'}, 69: {'artist': 'Bad Bunny & Tainy', 'title': 'Callaita'}, 70: {'artist': 'DJ Khaled Featuring Cardi B & 21 Savage', 'title': 'Wish Wish'}, 71: {'artist': 'Dan + Shay', 'title': 'All To Myself'}, 72: {'artist': 'Chase Rice', 'title': 'Eyes On You'}, 73: {'artist': 'Beyonce', 'title': 'Before I Let Go'}, 74: {'artist': 'Eric Church', 'title': 'Some Of It'}, 75: {'artist': 'Marshmello Featuring CHVRCHES', 'title': 'Here With Me'}, 76: {'artist': 'Lil Uzi Vert', 'title': 'Sanguine Paradise'}, 77: {'artist': 'Lunay, Daddy Yankee & Bad Bunny', 'title': 'Soltera'}, 78: {'artist': 'Florida Georgia Line', 'title': 'Talk You Out Of It'}, 79: {'artist': 'Yo Gotti Featuring Lil Baby', 'title': 'Put A Date On It'}, 80: {'artist': 'Eli Young Band', 'title': "Love Ain't"}, 81: {'artist': 'NLE Choppa', 'title': 'Shotta Flow'}, 82: {'artist': 'Pedro Capo X Farruko', 'title': 'Calma'}, 83: {'artist': 'Avicii', 'title': 'Heaven'}, 84: {'artist': 'The Chainsmokers & Bebe Rexha', 'title': 'Call You Mine'}, 85: {'artist': 'Billie Eilish', 'title': 'Ocean Eyes'}, 86: {'artist': 'Megan Thee Stallion', 'title': 'Big Ole Freak'}, 87: {'artist': 'Future', 'title': 'Please Tell Me'}, 88: {'artist': 'Cody Johnson', 'title': 'On My Way To You'}, 89: {'artist': 'SHAED', 'title': 'Trampoline'}, 90: {'artist': 'Chris Young', 'title': 'Raised On Country'}, 91: {'artist': 'Nicky Jam X Ozuna', 'title': 'Te Robare'}, 92: {'artist': 'Ozuna', 'title': 'Amor Genuino'}, 93: {'artist': 'Jonas Brothers', 'title': 'Only Human'}, 94: {'artist': 'Yella Beezy, Gucci Mane & Quavo', 'title': 'Bacc At It Again'}, 95: {'artist': 'Bryce Vine Featuring YG', 'title': 'La La Land'}, 96: {'artist': 'Juice WRLD', 'title': 'Robbery'}, 97: {'artist': 'Ozuna x Daddy Yankee x J Balvin x Farruko x Anuel AA', 'title': 'Baila Baila Baila'}, 98: {'artist': 'Future', 'title': 'XanaX Damage'}, 99: {'artist': 'Future', 'title': 'Government Official'}, 100: {'artist': 'Sech Featuring Darell', 'title': 'Otro Trago'}}

def read(conn, query):
    """Executes a Query against the specified connection and query params"""
    cursor = conn.cursor()
    cursor.execute(query)
    data = cursor.fetchall()
    print(data)
    return data


def top100Search(conn, top100):
    """Executes a query for the values in the top100 variable against the database specified within conn param"""
    results = []
    for items in top100: 
        topquery = "SELECT Title, Performer FROM Media WHERE Title ='" + top100[items]['title'].replace("'","''") + "'"
        temp = read(conn, topquery)
        if temp == []:
            continue
        else:    
            results.append(temp)
    return results 

def resultsparser(results):
    b = []
    for i in range(0, len(a)): 
        for j in range(0, len(a[i])):
            b.append(a[i][j])
    return b

The code is importing an external function (Get Config) which looks like this:

import pyodbc
import ConfigParser

def GetConfig(remoteServer):
   """Needs a specified server in config and returns back the ODBC parameters Server, Driver, Database, User, Password, Encrypt, TrustedServer, ConnectionTimeout""" 
   Config = ConfigParser.ConfigParser()
   Config.read('activeconfig.ini')
   Server = Config.get(remoteServer, 'Server')
   Driver = Config.get(remoteServer, 'Driver')
   Database = Config.get(remoteServer, "Database")
   User = Config.get(remoteServer, "User")
   Password = Config.get(remoteServer, "Password")
   Encrypt = Config.get(remoteServer, "Encrypt")
   TrustedServer = Config.get(remoteServer, "TrustedServer")
   ConnectionTimeout = Config.get(remoteServer, "ConnectionTimeout")
   return {"Server": Server,"Driver": Driver, "Database": Database, "User":User, "Password": Password, 
   "Encrypt": Encrypt, "TrustedServer": TrustedServer, "ConnectionTimeout": ConnectionTimeout}

The purpose of the code above is to get the database connection string, which is stored in a config.ini file. Which is structured like so and connects to a remote Azure Database, which is a replica of a media library:

[SomeServer]
Server:
Password:
Database:
User: 
Driver:
Encrypt: 
TrustedServer: 
ConnectionTimeout:

Overall, I'm wondering if my code is over specified or if any of my functions/variables should be migrated over into classes? I haven't included the scraper or the Pandas portion of the code, as I thought it would be a bit much but could include it if it's helpful.

\$\endgroup\$
2
\$\begingroup\$

A very strong suggestion: Fix the SQL injection in your code! This code should never ever be allowed into a production application.

Some suggestions:

  1. black can automatically format your code to be more idiomatic.
  2. flake8 with a strict complexity limit will give you more hints to write idiomatic Python:

    [flake8]
    max-complexity = 4
    ignore = W503,E203
    
  3. I would then recommend adding type hints and validating them using a strict mypy configuration:

    [mypy]
    check_untyped_defs = true
    disallow_untyped_defs = true
    ignore_missing_imports = true
    no_implicit_optional = true
    warn_redundant_casts = true
    warn_return_any = true
    warn_unused_ignores = true
    
  4. Naming is incredibly important for code maintainability. data is a well-known offender because it is completely meaningless. Every variable contains data, and unless you are handling arbitrary collections of completely unknown bytes there is a better name for the contents of that variable. b, for example, could at the very least be improved by calling it result - it's not going to conflict with the other result variable. It's not always easy to think of a better name, but it's time well spent.
  5. Your connection string could be formatted straight from the dictionary without duplicating the names (untested):

    "".join([f"{key}={value};" for key, value in connection_configuration.items()])
    
  6. conn and top100 are both shadowed by the top100Search parameters - they have the same name. This is a bug magnet.
  7. GetConfig might as well return the entire dictionary - the code is much simpler that way, and anything that's not needed in the configuration would cause a handy error or would simply be ignored.
  8. Values like Location would be better off as configuration - it has no bearing on the logic of this code.
  9. You have a bug in your code - resultparser takes results but seems to think it's called a in the code.
\$\endgroup\$

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.