diff --git a/.pylintrc b/.pylintrc index 7b9da25..2932039 100644 --- a/.pylintrc +++ b/.pylintrc @@ -145,7 +145,10 @@ disable=print-statement, global-statement, bad-continuation, too-few-public-methods, - no-else-return + no-else-return, + consider-using-f-string, + use-implicit-booleaness-not-comparison, + duplicate-code # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option diff --git a/Dockerfile b/Dockerfile index cc22d8b..4b9ffb8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ # We do a multi-stage build (requires Docker >= 17.05) to install everything, then copy it all # to the final target image. -FROM centos:8 AS base +FROM rockylinux:8 AS base # update packages and install Python RUN dnf -y upgrade-minimal && \ diff --git a/asl_articles/__init__.py b/asl_articles/__init__.py index fd1ba88..e938e9f 100644 --- a/asl_articles/__init__.py +++ b/asl_articles/__init__.py @@ -75,7 +75,7 @@ _load_config( _cfg, _fname, "Debug" ) # initialize logging _fname = os.path.join( config_dir, "logging.yaml" ) if os.path.isfile( _fname ): - with open( _fname, "r" ) as fp: + with open( _fname, "r", encoding="utf-8" ) as fp: logging.config.dictConfig( yaml.safe_load( fp ) ) else: # stop Flask from logging every request :-/ diff --git a/asl_articles/db_report.py b/asl_articles/db_report.py index 498fdab..901ff2e 100644 --- a/asl_articles/db_report.py +++ b/asl_articles/db_report.py @@ -65,16 +65,15 @@ def check_db_link(): """Check if a link appears to be working.""" url = request.args.get( "url" ) try: - resp = urllib.request.urlopen( - urllib.request.Request( url, method="HEAD" ) - ) + req = urllib.request.Request( url, method="HEAD" ) + with urllib.request.urlopen( req ) as resp: + resp_code = resp.code except urllib.error.URLError as ex: - code = getattr( ex, "code", None ) - if code: - abort( code ) - abort( 400 ) - if resp.code != 200: - abort( resp.code ) + resp_code = getattr( ex, "code", None ) + if not resp_code: + resp_code = 400 + if resp_code != 200: + abort( resp_code ) return "ok" # --------------------------------------------------------------------- diff --git a/asl_articles/images.py b/asl_articles/images.py index 7d9765e..42a8e58 100644 --- a/asl_articles/images.py +++ b/asl_articles/images.py @@ -21,5 +21,5 @@ def get_image( image_type, image_id ): abort( 404 ) return send_file( io.BytesIO( img.image_data ), - attachment_filename = img.image_filename # nb: so that Flask can set the MIME type + download_name = img.image_filename # nb: so that Flask can set the MIME type ) diff --git a/asl_articles/main.py b/asl_articles/main.py index 459c2b1..34f83b9 100644 --- a/asl_articles/main.py +++ b/asl_articles/main.py @@ -1,7 +1,5 @@ """ Main handlers. """ -from flask import request - from asl_articles import app # --------------------------------------------------------------------- @@ -10,11 +8,3 @@ from asl_articles import app def ping(): """Let the caller know we're alive (for testing porpoises).""" return "pong" - -# --------------------------------------------------------------------- - -@app.route( "/shutdown" ) -def shutdown(): - """Shutdown the server (for testing porpoises).""" - request.environ.get( "werkzeug.server.shutdown" )() - return "" diff --git a/asl_articles/tests/test_articles.py b/asl_articles/tests/test_articles.py index 0957980..00af88f 100644 --- a/asl_articles/tests/test_articles.py +++ b/asl_articles/tests/test_articles.py @@ -278,8 +278,9 @@ def test_images( webdriver, flask_app, dbconn ): #pylint: disable=too-many-state btn = find_child( ".row.image .remove-image", dlg ) assert btn.is_displayed() # make sure the article's image is correct - resp = urllib.request.urlopen( image_url ).read() - assert resp == open( expected, "rb" ).read() + with urllib.request.urlopen( image_url ) as resp: + with open( expected, "rb" ) as fp: + assert resp.read() == fp.read() else: # make sure there is no image img = find_child( ".row.image img.image", dlg ) @@ -290,7 +291,8 @@ def test_images( webdriver, flask_app, dbconn ): #pylint: disable=too-many-state # make sure the article's image is not available url = flask_app.url_for( "get_image", image_type="article", image_id=article_id ) try: - resp = urllib.request.urlopen( url ) + with urllib.request.urlopen( url ): + pass assert False, "Should never get here!" except urllib.error.HTTPError as ex: assert ex.code == 404 @@ -350,7 +352,8 @@ def test_parent_publisher( webdriver, flask_app, dbconn ): # check that the parent publication was updated in the database article_id = sr.get_attribute( "testing--article_id" ) url = flask_app.url_for( "get_article", article_id=article_id ) - article = json.load( urllib.request.urlopen( url ) ) + with urllib.request.urlopen( url ) as resp: + article = json.load( resp ) if expected_parent: if article["pub_id"] != expected_parent[0]: return None diff --git a/asl_articles/tests/test_authors.py b/asl_articles/tests/test_authors.py index 5bc67c2..f1b3ce2 100644 --- a/asl_articles/tests/test_authors.py +++ b/asl_articles/tests/test_authors.py @@ -92,5 +92,6 @@ def _check_authors( flask_app, all_authors, expected ): # check the authors in the database url = flask_app.url_for( "get_authors" ) - authors = json.load( urllib.request.urlopen( url ) ) + with urllib.request.urlopen( url ) as resp: + authors = json.load( resp ) assert set( a["author_name"] for a in authors.values() ) == all_authors diff --git a/asl_articles/tests/test_import_roar_scenarios.py b/asl_articles/tests/test_import_roar_scenarios.py index 6c8b1bb..c0c20a4 100644 --- a/asl_articles/tests/test_import_roar_scenarios.py +++ b/asl_articles/tests/test_import_roar_scenarios.py @@ -18,7 +18,8 @@ def test_import_roar_scenarios( dbconn ): # initialize session = init_tests( None, None, dbconn ) roar_fname = os.path.join( os.path.split(__file__)[0], "fixtures/roar-scenarios.json" ) - roar_data = json.load( open( roar_fname, "r" ) ) + with open( roar_fname, "r", encoding="utf-8" ) as fp: + roar_data = json.load( fp ) # do the first import _do_import( dbconn, session, roar_fname, diff --git a/asl_articles/tests/test_publications.py b/asl_articles/tests/test_publications.py index db2be13..18bdc3e 100644 --- a/asl_articles/tests/test_publications.py +++ b/asl_articles/tests/test_publications.py @@ -246,8 +246,9 @@ def test_images( webdriver, flask_app, dbconn ): #pylint: disable=too-many-state btn = find_child( ".row.image .remove-image", dlg ) assert btn.is_displayed() # make sure the publication's image is correct - resp = urllib.request.urlopen( image_url ).read() - assert resp == open( expected, "rb" ).read() + with urllib.request.urlopen( image_url ) as resp: + with open( expected, "rb" ) as fp: + assert resp.read() == fp.read() else: # make sure there is no image img = find_child( ".row.image img.image", dlg ) @@ -258,7 +259,8 @@ def test_images( webdriver, flask_app, dbconn ): #pylint: disable=too-many-state # make sure the publication's image is not available url = flask_app.url_for( "get_image", image_type="publication", image_id=pub_id ) try: - resp = urllib.request.urlopen( url ) + with urllib.request.urlopen( url ): + pass assert False, "Should never get here!" except urllib.error.HTTPError as ex: assert ex.code == 404 @@ -318,7 +320,8 @@ def test_parent_publisher( webdriver, flask_app, dbconn ): # check that the parent publisher was updated in the database pub_id = sr.get_attribute( "testing--pub_id" ) url = flask_app.url_for( "get_publication", pub_id=pub_id ) - pub = json.load( urllib.request.urlopen( url ) ) + with urllib.request.urlopen( url ) as resp: + pub = json.load( resp ) if expected_parent: if pub["publ_id"] != expected_parent[0]: return None @@ -672,8 +675,11 @@ def test_default_image( webdriver, flask_app, dbconn ): f: os.path.join( os.path.split(__file__)[0], "fixtures/images/"+f ) for f in images } + def read_image_data( fname ): + with open( fname, "rb" ) as fp: + return fp.read() image_data = { - f: open( image_fnames[f], "rb" ).read() + f: read_image_data( image_fnames[f] ) for f in images } @@ -690,8 +696,8 @@ def test_default_image( webdriver, flask_app, dbconn ): if img: assert expected image_url = img.get_attribute( "src" ) - resp = urllib.request.urlopen( image_url ).read() - assert resp == image_data[ expected ] + with urllib.request.urlopen( image_url ) as resp: + assert resp.read() == image_data[ expected ] else: assert not expected diff --git a/asl_articles/tests/test_publishers.py b/asl_articles/tests/test_publishers.py index fdb4f6b..c3fd963 100644 --- a/asl_articles/tests/test_publishers.py +++ b/asl_articles/tests/test_publishers.py @@ -176,8 +176,9 @@ def test_images( webdriver, flask_app, dbconn ): #pylint: disable=too-many-state btn = find_child( ".row.image .remove-image", dlg ) assert btn.is_displayed() # make sure the publisher's image is correct - resp = urllib.request.urlopen( image_url ).read() - assert resp == open(expected,"rb").read() + with urllib.request.urlopen( image_url ) as resp: + with open( expected, "rb" ) as fp: + assert resp.read() == fp.read() else: # make sure there is no image img = find_child( ".row.image img.image", dlg ) @@ -188,7 +189,8 @@ def test_images( webdriver, flask_app, dbconn ): #pylint: disable=too-many-state # make sure the publisher's image is not available url = flask_app.url_for( "get_image", image_type="publisher", image_id=publ_id ) try: - resp = urllib.request.urlopen( url ) + with urllib.request.urlopen( url ): + pass assert False, "Should never get here!" except urllib.error.HTTPError as ex: assert ex.code == 404 diff --git a/asl_articles/tests/test_scenarios.py b/asl_articles/tests/test_scenarios.py index 25b32ba..9bcd315 100644 --- a/asl_articles/tests/test_scenarios.py +++ b/asl_articles/tests/test_scenarios.py @@ -104,7 +104,8 @@ def _check_scenarios( flask_app, all_scenarios, expected ): # check the scenarios in the database url = flask_app.url_for( "get_scenarios" ) - scenarios = json.load( urllib.request.urlopen( url ) ) + with urllib.request.urlopen( url ) as resp: + scenarios = json.load( resp ) assert set( _make_scenario_display_name(a) for a in scenarios.values() ) == all_scenarios def _make_scenario_display_name( scenario ): diff --git a/asl_articles/tests/test_tags.py b/asl_articles/tests/test_tags.py index 5091503..442f7f8 100644 --- a/asl_articles/tests/test_tags.py +++ b/asl_articles/tests/test_tags.py @@ -145,10 +145,12 @@ def _check_tags( flask_app, expected ): #pylint: disable=too-many-locals if sr.text.startswith( "publication" ): pub_id = sr.get_attribute( "testing--pub_id" ) url = flask_app.url_for( "get_publication", pub_id=pub_id ) - pub = json.load( urllib.request.urlopen( url ) ) + with urllib.request.urlopen( url ) as resp: + pub = json.load( resp ) assert expected[ pub["pub_name"] ] == fixup_tags( pub["pub_tags"] ) elif sr.text.startswith( "article" ): article_id = sr.get_attribute( "testing--article_id" ) url = flask_app.url_for( "get_article", article_id=article_id ) - article = json.load( urllib.request.urlopen( url ) ) + with urllib.request.urlopen( url ) as resp: + article = json.load( resp ) assert expected[ article["article_title"] ] == fixup_tags( article["article_tags"] ) diff --git a/asl_articles/tests/utils.py b/asl_articles/tests/utils.py index e002bce..76800ff 100644 --- a/asl_articles/tests/utils.py +++ b/asl_articles/tests/utils.py @@ -49,7 +49,8 @@ def init_tests( webdriver, flask_app, dbconn, **kwargs ): # re-initialize the search engine if flask_app: url = flask_app.url_for( "init_search_for_test" ) - _ = urllib.request.urlopen( url ).read() + with urllib.request.urlopen( url ) as resp: + _ = resp.read() # initialize the documents directory dname = kwargs.pop( "docs", None ) @@ -70,7 +71,10 @@ def init_tests( webdriver, flask_app, dbconn, **kwargs ): if to_bool( kwargs.pop( "disable_confirm_discard_changes", True ) ): kwargs[ "disable_confirm_discard_changes" ] = 1 webdriver.get( webdriver.make_url( "", **kwargs ) ) - wait_for_elem( 2, "#search-form" ) + # FUDGE! Since we switched from running the test Flask server with app.run() to make_server().serve_forever(), + # stopping and starting the server seems to be much quicker, but refreshing the page can be slower when + # running multiple tests :shrug: + wait_for_elem( 10, "#search-form" ) return session @@ -83,7 +87,8 @@ def load_fixtures( session, fname ): if fname: dname = os.path.join( os.path.split(__file__)[0], "fixtures/" ) fname = os.path.join( dname, fname ) - data = json.load( open( fname, "r" ) ) + with open( fname, "r", encoding="utf-8" ) as fp: + data = json.load( fp ) else: data = {} @@ -318,21 +323,21 @@ def wait_for_not_elem( timeout, sel ): def find_child( sel, parent=None ): """Find a child element.""" try: - return (parent if parent else _webdriver).find_element_by_css_selector( sel ) + return (parent if parent else _webdriver).find_element( By.CSS_SELECTOR, sel ) except NoSuchElementException: return None def find_children( sel, parent=None ): """Find child elements.""" try: - return (parent if parent else _webdriver).find_elements_by_css_selector( sel ) + return (parent if parent else _webdriver).find_elements( By.CSS_SELECTOR, sel ) except NoSuchElementException: return None def find_parent_by_class( elem, class_name ): """Find a parent element with the specified class.""" while True: - elem = elem.find_element_by_xpath( ".." ) + elem = elem.find_element( By.XPATH, ".." ) if not elem: return None classes = set( elem.get_attribute( "class" ).split() ) @@ -497,7 +502,8 @@ def call_with_retry( func, expected_exceptions, max_retries=10, delay=0.1 ): def change_image( dlg, fname ): """Click on an image to change it.""" # NOTE: This is a bit tricky since we started overlaying the image with the "remove image" icon :-/ - data = base64.b64encode( open( fname, "rb" ).read() ) + with open( fname, "rb" ) as fp: + data = base64.b64encode( fp.read() ) data = "{}|{}".format( os.path.split(fname)[1], data.decode("ascii") ) elem = find_child( ".row.image img.image", dlg ) _webdriver.execute_script( "arguments[0].scrollTo( 0, 0 )", find_child( ".MuiDialogContent-root", dlg ) ) diff --git a/conftest.py b/conftest.py index dbcc54c..8cdc35a 100644 --- a/conftest.py +++ b/conftest.py @@ -9,6 +9,7 @@ from urllib.error import URLError import pytest import flask +import werkzeug import sqlalchemy from flask_sqlalchemy import SQLAlchemy import alembic @@ -99,19 +100,22 @@ def flask_app( request ): # the *configured* database connection string (since it will fail to start if there's a problem). asl_articles._disable_db_startup = True #pylint: disable=protected-access # yup - make it so + server = werkzeug.serving.make_server( + _FLASK_SERVER_URL[0], _FLASK_SERVER_URL[1], + app, threaded=True + ) thread = threading.Thread( - target = lambda: app.run( - host=_FLASK_SERVER_URL[0], port=_FLASK_SERVER_URL[1], - use_reloader=False - ) + target = server.serve_forever, + daemon=True ) thread.start() # wait for the server to start up def is_ready(): """Try to connect to the Flask server.""" try: - resp = urllib.request.urlopen( app.url_for( "ping" ) ).read() - assert resp == b"pong" + url = app.url_for( "ping" ) + with urllib.request.urlopen( url ) as resp: + assert resp.read() == b"pong" return True except URLError: return False @@ -125,7 +129,7 @@ def flask_app( request ): finally: # shutdown the local Flask server if not flask_url: - urllib.request.urlopen( app.url_for("shutdown") ).read() + server.shutdown() thread.join() # --------------------------------------------------------------------- @@ -142,10 +146,7 @@ def webdriver( request ): options = wb.FirefoxOptions() if headless: options.add_argument( "--headless" ) #pylint: disable=no-member - driver = wb.Firefox( - options = options, - service_log_path = os.path.join( tempfile.gettempdir(), "geckodriver.log" ) - ) + driver = wb.Firefox( options=options ) elif driver == "chrome": options = wb.ChromeOptions() if headless: diff --git a/requirements-dev.txt b/requirements-dev.txt index 7d96d7f..d6870ca 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,5 +1,5 @@ -pytest==6.2.1 -selenium==3.141.0 -pylint==2.6.0 +pytest==6.2.5 +selenium==4.1.0 +pylint==2.12.2 pylint-flask-sqlalchemy==0.2.0 pytest-pylint==0.18.0 diff --git a/requirements.txt b/requirements.txt index cc6bb4e..b429219 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,13 +1,13 @@ # python 3.8.7 -flask==1.1.2 +flask==2.0.3 # NOTE: Newer versions of SQLAlchemy contain a change that breaks Flask-SQLALchemy :-/ # https://stackoverflow.com/a/66652728 # This wasn't a problem on vm-linux-dev, but manifested itself on the rPi4 (probably because # the virtualenv on vm-linux-dev was built before this became a problem). flask-sqlalchemy==2.5.1 -psycopg2-binary==2.8.6 -alembic==1.4.3 -pyyaml==5.3.1 -lxml==4.6.2 +psycopg2-binary==2.9.3 +alembic==1.7.6 +pyyaml==6.0 +lxml==4.8.0 waitress==2.0.0 diff --git a/run_server.py b/run_server.py index 62e78fd..a14b8c5 100755 --- a/run_server.py +++ b/run_server.py @@ -48,7 +48,8 @@ def _force_init(): host = host[:-1] url = "{}:{}{}".format( host, flask_port, url ) # make the request - _ = urllib.request.urlopen( url ).read() + with urllib.request.urlopen( url ) as resp: + _ = resp.read() except Exception as ex: #pylint: disable=broad-except print( "WARNING: Startup ping failed: {}".format( ex ) ) threading.Thread( target=_force_init ).start() diff --git a/setup.py b/setup.py index 6ff1be9..fadd3aa 100644 --- a/setup.py +++ b/setup.py @@ -16,11 +16,12 @@ def parse_requirements( fname ): """Parse a requirements file.""" lines = [] fname = os.path.join( os.path.split(__file__)[0], fname ) - for line in open(fname,"r"): - line = line.strip() - if line == "" or line.startswith("#"): - continue - lines.append( line ) + with open( fname, "r", encoding="utf-8" ) as fp: + for line in fp: + line = line.strip() + if line == "" or line.startswith("#"): + continue + lines.append( line ) return lines # --------------------------------------------------------------------- diff --git a/tools/import_roar_scenarios.py b/tools/import_roar_scenarios.py index e1c166a..00d142c 100755 --- a/tools/import_roar_scenarios.py +++ b/tools/import_roar_scenarios.py @@ -62,7 +62,8 @@ def import_roar_scenarios( dbconn, roar_data, progress=None ): # load the ROAR scenarios if isinstance( roar_data, str ): log_progress( "Loading scenarios: {}", roar_data ) - roar_data = json.load( open( roar_data, "r" ) ) + with open( roar_data, "r", encoding="utf-8" ) as fp: + roar_data = json.load( fp ) else: assert isinstance( roar_data, dict ) log_progress( "- Last updated: {}".format( roar_data.get("_lastUpdated_","(unknown)") ) )