Given a text file containing a list of links, I wrote some code to download them, one by one, from YouTube, using either YoutubeMP3 or Convert2mp3 logging any songs that failed to download.The code with Youtube-mp3 is significantly faster than Convert2mp3 but less accurate (some songs fail for unknown reasons, even manually), due to the nature of the websites, but any tips on improving performance would be greatly appreciated.Main.pyimport sysfrom tkinter.filedialog import askdirectory, askopenfilenamefrom selenium import webdriverfrom selenium.common.exceptions import NoSuchWindowException, WebDriverExceptionfrom downloader import downloaderfrom downloader import loggerFIREFOX_DRIVER_PATH =  # A path need to coded here.CONVERTER2MP3 = http://convert2mp3.net/en/index.phpMP3_CONVERTER = http://www.youtube-mp3.org/WEBSITE = {1: CONVERTER2MP3, 2: MP3_CONVERTER}def get_firefox_profile(download_directory):    profile = webdriver.FirefoxProfile()    profile.set_preference(dom.popup_maximum, 0)    profile.set_preference(browser.download.folderList, 2)    profile.set_preference(browser.download.dir, download_directory)    profile.set_preference(browser.download.panel.shown, False)    profile.set_preference(privacy.popups.showBrowserMessage, False)    profile.set_preference(browser.download.manager.showWhenStarting, False)    profile.set_preference(browser.helperApps.neverAsk.saveToDisk, .mp3 audio/mpeg)    return profiledef get_user_choice():    input(press enter when ready to choose the list of songs)    songs_list = askopenfilename(initialdir='.')    if not songs_list:        raise ValueError(You did not choose the songs list file.)    if not songs_list.endswith(txt):        raise ValueError(A text file must be chosen.)    input(press enter when ready to choose the download directory)    download_directory = askdirectory(initialdir='.')    if not download_directory:        raise ValueError(You did not choose the download directory.)    return songs_list, download_directorydef main():    if len(sys.argv) != 2:        print(Incorrect number of arguments)        print(You need to choose one of the following modes:\n1 - Downloading the songs from converter2mp3.\n2 -             Downloading the songs from mp3 converter.)        sys.exit(1)    mode = sys.argv[1]    try:        songs_list, download_directory = get_user_choice()    except ValueError as e:        print(e)        logger.exception(e)        sys.exit(1)    print(Starting to download songs...)    try:        profile = get_firefox_profile(download_directory)        driver = webdriver.Firefox(firefox_profile=profile, executable_path=FIREFOX_DRIVER_PATH)        driver.get(WEBSITE[mode])        downloader(driver, mode, songs_list)    except (WebDriverException, NoSuchWindowException) as e:        logger.exception(e)        sys.exit(1)    else:        driver.close()        sys.exit(0)if __name__ == '__main__':    main()Downloader.pyimport loggingimport timefrom selenium.common.exceptions import NoSuchElementException, TimeoutExceptionfrom selenium.webdriver.common.by import Byfrom selenium.webdriver.remote.remote_connection import LOGGERfrom selenium.webdriver.support import expected_conditions as ECfrom selenium.webdriver.support.ui import WebDriverWaitLOGGER.setLevel(logging.WARNING)logger = logging.getLogger(__name__)logging.basicConfig(filename=failed_songs.log, level=logging.DEBUG)# MP3_CONVERTERENTER_SONG_NAME = document.getElementById(youtube-url).value={};SUBMIT_SONG = document.getElementById(submit).click();PRESS_DOWNLOAD = for(child in document.getElementById(dl_link).childNodes)if (document.getElementById(dl_link).childNodes.hasOwnProperty(child))if(document.getElementById(dl_link).childNodes[child].style.display==){document.getElementById(dl_link).childNodes[child].click();break;}# CONVERTER2MP3INSERT_SONG_NAME = document.getElementById(urlinput).value={};CONVERT_SONG = document.getElementsByTagName(button)[1].click();SKIP_TAGS = document.getElementsByTagName(a)[12].click();DOWNLOAD_SONG = document.getElementsByTagName(a)[9].click();CONVERT_NEXT = document.getElementsByTagName(span)[0].click();def download_mp3_converter(driver, song):    driver.execute_script(ENTER_SONG_NAME.format(song))    driver.execute_script(SUBMIT_SONG)    WebDriverWait(driver, 5).until(EC.visibility_of_element_located((By.ID, dl_link)))    driver.execute_script(PRESS_DOWNLOAD)def download_converter2mp3(driver, song):    def execute(web_driver, command):        web_driver.execute_script(command)        time.sleep(3)    driver.execute_script(INSERT_SONG_NAME.format(song))    driver.execute_script(CONVERT_SONG)    WebDriverWait(driver, 20).until(EC.element_to_be_clickable((By.ID, advancedtags_btn)))    execute(driver, SKIP_TAGS)    execute(driver, DOWNLOAD_SONG)    execute(driver, CONVERT_NEXT)DOWNLOAD_FUNCTIONS = {1: download_converter2mp3, 2: download_mp3_converter}def downloader(driver, mode, songs_list):    download_song = DOWNLOAD_FUNCTIONS[mode]    with open(songs_list, r, encoding=utf-8-sig) as songs:        for song in songs:            try:                download_song(driver, song.strip(\n))            except (TimeoutException, NoSuchElementException):                logger.warning( Failed to download:  + song.strip(\n))This is my first project not done to the instruction of or under the supervision of anyone, so code review is important to me.Points I am unsure about:Organization, naming and conventions, the placement of my many constants, the hardcoded Javascript.Downloader.py my nested execute wrapper function, it can easily be replaced and although I like it better, it may be superfluous.I could have logged every activity going on, but since logging is not the main purpose of the code, I removed the info loggings and kept only the important ones.Besides that, any other opinion is much appreciated, including topics I didn't take into consideration like security.
Download a list of songs from Youtube using Selenium
python;performance;audio;selenium;youtube
Here are some first impressions and thoughts.Code OrganizationMove all selenium specific code to the downloader. I think your main script should only be responsible for getting the input parameters and then invoke the downloader which hides the way it does the downloading internally. I would move the profile preparation and the webdriver initialization to the downloader.  to continue on the topic of your downloader. You should consider creating a Downloader class. One of the benefits of having a class is that you may keep your driver as an instance variable. And, you may also define the WebDriverWait instance once and reuse instead of re-instantiating it every time you need to wait. Code Styleconsider using multi-line strings for your JS scriptsadd meaningful docstrings to your modules and functionsyou can improve the get_firefox_profile() function by pre-defining a dictionary of preferences and setting them at once. Something like:FIREFOX_PREFERENCES = {    dom.popup_maximum: 0,    browser.download.folderList: 2,    browser.download.panel.shown: False,    privacy.popups.showBrowserMessage: False,    browser.download.manager.showWhenStarting: False,    browser.helperApps.neverAsk.saveToDisk: .mp3 audio/mpeg,}def get_firefox_profile(download_directory):    profile = webdriver.FirefoxProfile()    profile.default_preferences.update(FIREFOX_PREFERENCES)    profile.set_preference(browser.download.dir, download_directory)    return profileReliabilityFragile locators. Even though you are defining JS scripts as locators, the locators themselves are fragile - for instance, relying on the a element to be 12th or 9th in the DOM tree may be broken easily.To continue on this topic, I would expect the downloader to use Selenium-specific location techniques instead of doing it in pure JavaScript. You could have used CSS selectors or XPath expressions based on meaningful element classes or other attributes. Though, I suspect that you did that intentionally to minimize the number of Selenium commands processed via JSON over HTTP. If this is the case, I think you sacrifice reliability. And, also make things more difficult to understand. For instance, it is not immediately obvious what PRESS_DOWNLOAD script does.avoid using time.sleep() - it generally leads to waiting more than needed in 95% of cases and waiting less in 5%. Ideally, you should use Explicit Waits if it is at all possible in your situation.  Other Random ThoughtsImprove reading command-line arguments. You don't have a lot of CLI arguments, but you can still improve the way you do it - consider switching to argparse which will help to make this part of the code more readable and conciseIf the project and the browser interaction logic will continue to grow, consider using a Page Object patternselenium brings you a lot of overhead, please see if you can either find an mp3 converter that has a public API, or see if you can use existing two without using a real browser