diff options
| -rw-r--r-- | quality_control/checks.py | 15 | ||||
| -rw-r--r-- | quality_control/parsing.py | 17 | ||||
| -rw-r--r-- | tests/uploader/phenotypes/test_misc.py | 108 | ||||
| -rw-r--r-- | tests/uploader/publications/test_misc.py | 1 | ||||
| -rw-r--r-- | tests/uploader/test_parse.py | 3 | ||||
| -rw-r--r-- | uploader/__init__.py | 11 | ||||
| -rw-r--r-- | uploader/authorisation.py | 2 | ||||
| -rw-r--r-- | uploader/background_jobs.py | 27 | ||||
| -rw-r--r-- | uploader/jobs.py | 9 | ||||
| -rw-r--r-- | uploader/monadic_requests.py | 16 | ||||
| -rw-r--r-- | uploader/platforms/models.py | 3 | ||||
| -rw-r--r-- | uploader/route_utils.py | 3 |
12 files changed, 164 insertions, 51 deletions
diff --git a/quality_control/checks.py b/quality_control/checks.py index bdfd12b..bb05e31 100644 --- a/quality_control/checks.py +++ b/quality_control/checks.py @@ -52,12 +52,15 @@ def decimal_places_pattern(mini: int, maxi: Optional[int] = None) -> re.Pattern: + r")$" ) -def decimal_points_error(filename: str,# pylint: disable=[too-many-arguments] - lineno: int, - field: str, - value: str, - mini: int, - maxi: Optional[int] = None) -> Optional[InvalidValue]: +def decimal_points_error( + # pylint: disable=[too-many-arguments, too-many-positional-arguments] + filename: str, + lineno: int, + field: str, + value: str, + mini: int, + maxi: Optional[int] = None +) -> Optional[InvalidValue]: """ Check that 'value' in a decimal number with the appropriate decimal places. """ diff --git a/quality_control/parsing.py b/quality_control/parsing.py index f1d21fc..7a8185d 100644 --- a/quality_control/parsing.py +++ b/quality_control/parsing.py @@ -104,23 +104,22 @@ def collect_errors( if line_number == 1: consistent_columns_checker = make_column_consistency_checker( filename, line) - for error in __process_errors__( - filename, line_number, line, - partial(header_errors, strains=strains), - errors): - yield error + yield from __process_errors__( + filename, line_number, line, + partial(header_errors, strains=strains), + errors) if line_number != 1: - col_consistency_error = consistent_columns_checker(line_number, line) + col_consistency_error = consistent_columns_checker(# pylint: disable=[possibly-used-before-assignment] + line_number, line) if col_consistency_error: yield col_consistency_error - for error in __process_errors__( + yield from __process_errors__( filename, line_number, line, ( average_errors if filetype == FileType.AVERAGE else se_errors), - errors): - yield error + errors) if update_progress: update_progress(line_number, line) diff --git a/tests/uploader/phenotypes/test_misc.py b/tests/uploader/phenotypes/test_misc.py index c0261aa..cf475ad 100644 --- a/tests/uploader/phenotypes/test_misc.py +++ b/tests/uploader/phenotypes/test_misc.py @@ -218,12 +218,54 @@ __sample_db_phenotypes_data__ = ( } }), __sample_db_phenotypes_data__, - ({"PhenotypeId": 4, "xref_id": 10001, "DataId": 8967043, "StrainId": 4, "StrainName": "BXD1", "value": 77.2}, - {"PhenotypeId": 15, "xref_id": 10003, "DataId": 8967045, "StrainId": 6, "StrainName": "BXD5", "value": 503}, - {"PhenotypeId": 15, "xref_id": 10003, "DataId": 8967045, "StrainId": 7, "StrainName": "BXD6", "value": 903}, - {"PhenotypeId": 20, "xref_id": 10004, "DataId": 8967046, "StrainId": 3, "StrainName": "DBA/2J", "value": 1}, - {"PhenotypeId": 20, "xref_id": 10004, "DataId": 8967046, "StrainId": 4, "StrainName": "BXD1", "value": 8}, - {"PhenotypeId": 20, "xref_id": 10004, "DataId": 8967046, "StrainId": 5, "StrainName": "BXD2", "value": 9})), + ({ + "PhenotypeId": 4, + "xref_id": 10001, + "DataId": 8967043, + "StrainId": 4, + "StrainName": "BXD1", + "value": 77.2 + }, + { + "PhenotypeId": 15, + "xref_id": 10003, + "DataId": 8967045, + "StrainId": 6, + "StrainName": "BXD5", + "value": 503 + }, + { + "PhenotypeId": 15, + "xref_id": 10003, + "DataId": 8967045, + "StrainId": 7, + "StrainName": "BXD6", + "value": 903 + }, + { + "PhenotypeId": 20, + "xref_id": 10004, + "DataId": 8967046, + "StrainId": 3, + "StrainName": "DBA/2J", + "value": 1 + }, + { + "PhenotypeId": 20, + "xref_id": 10004, + "DataId": 8967046, + "StrainId": 4, + "StrainName": "BXD1", + "value": 8 + }, + { + "PhenotypeId": 20, + "xref_id": 10004, + "DataId": 8967046, + "StrainId": 5, + "StrainName": "BXD2", + "value": 9 + })), # Changes — with deletions (({ @@ -292,12 +334,54 @@ __sample_db_phenotypes_data__ = ( } }), __sample_db_phenotypes_data__, - ({"PhenotypeId": 4, "xref_id": 10001, "DataId": 8967043, "StrainId": 4, "StrainName": "BXD1", "value": None}, - {"PhenotypeId": 15, "xref_id": 10003, "DataId": 8967045, "StrainId": 6, "StrainName": "BXD5", "value": None}, - {"PhenotypeId": 15, "xref_id": 10003, "DataId": 8967045, "StrainId": 7, "StrainName": "BXD6", "value": None}, - {"PhenotypeId": 20, "xref_id": 10004, "DataId": 8967046, "StrainId": 3, "StrainName": "DBA/2J", "value": 15}, - {"PhenotypeId": 20, "xref_id": 10004, "DataId": 8967046, "StrainId": 4, "StrainName": "BXD1", "value": None}, - {"PhenotypeId": 20, "xref_id": 10004, "DataId": 8967046, "StrainId": 5, "StrainName": "BXD2", "value": 24})))) + ({ + "PhenotypeId": 4, + "xref_id": 10001, + "DataId": 8967043, + "StrainId": 4, + "StrainName": "BXD1", + "value": None + }, + { + "PhenotypeId": 15, + "xref_id": 10003, + "DataId": 8967045, + "StrainId": 6, + "StrainName": "BXD5", + "value": None + }, + { + "PhenotypeId": 15, + "xref_id": 10003, + "DataId": 8967045, + "StrainId": 7, + "StrainName": "BXD6", + "value": None + }, + { + "PhenotypeId": 20, + "xref_id": 10004, + "DataId": 8967046, + "StrainId": 3, + "StrainName": "DBA/2J", + "value": 15 + }, + { + "PhenotypeId": 20, + "xref_id": 10004, + "DataId": 8967046, + "StrainId": 4, + "StrainName": "BXD1", + "value": None + }, + { + "PhenotypeId": 20, + "xref_id": 10004, + "DataId": 8967046, + "StrainId": 5, + "StrainName": "BXD2", + "value": 24 + })))) def test_phenotypes_data_differences(filedata, dbdata, expected): """Test differences are computed correctly.""" assert phenotypes_data_differences(filedata, dbdata) == expected diff --git a/tests/uploader/publications/test_misc.py b/tests/uploader/publications/test_misc.py index 7a52941..8c7e567 100644 --- a/tests/uploader/publications/test_misc.py +++ b/tests/uploader/publications/test_misc.py @@ -63,5 +63,6 @@ from uploader.publications.misc import publications_differences {"PhenotypeId": 1, "xref_id": 10004, "PublicationId": None, "PubMed_ID": None})))) def test_publications_differences(filedata, dbdata, pubmed2pubidmap, expected): + """Test publication differences — flesh out description…""" assert publications_differences( filedata, dbdata, pubmed2pubidmap) == expected diff --git a/tests/uploader/test_parse.py b/tests/uploader/test_parse.py index 076c47c..20c75b7 100644 --- a/tests/uploader/test_parse.py +++ b/tests/uploader/test_parse.py @@ -8,7 +8,8 @@ from uploader.jobs import job, jobsnamespace from tests.conftest import uploadable_file_object -def test_parse_with_existing_uploaded_file(#pylint: disable=[too-many-arguments] +def test_parse_with_existing_uploaded_file( + #pylint: disable=[too-many-arguments,too-many-positional-arguments] client, db_url, redis_url, diff --git a/uploader/__init__.py b/uploader/__init__.py index b986c81..97b9af5 100644 --- a/uploader/__init__.py +++ b/uploader/__init__.py @@ -3,13 +3,17 @@ import os import sys import logging from pathlib import Path +from typing import Optional from flask import Flask, request -from flask_session import Session + from cachelib import FileSystemCache from gn_libs import jobs as gnlibs_jobs +from flask_session import Session + + from uploader.oauth2.client import user_logged_in, authserver_authorise_uri from . import session @@ -66,12 +70,15 @@ def setup_modules_logging(app_logger): logging.getLogger("uploader.publications.models").setLevel(loglevel) -def create_app(config: dict = {}): +def create_app(config: Optional[dict] = None): """The application factory. config: dict Useful to override settings in the settings files and environment especially in environments such as testing.""" + if config is None: + config = {} + app = Flask(__name__) ### BEGIN: Application configuration diff --git a/uploader/authorisation.py b/uploader/authorisation.py index bc950d8..3cf3585 100644 --- a/uploader/authorisation.py +++ b/uploader/authorisation.py @@ -48,7 +48,7 @@ def require_token(func: Callable) -> Callable: """ def __invalid_token__(_whatever): logging.debug("==========> Failure log: %s", _whatever) - raise Exception( + raise Exception(# pylint: disable=[broad-exception-raised] "You attempted to access a feature of the system that requires " "authorisation. Unfortunately, we could not verify you have the " "appropriate authorisation to perform the action you requested. " diff --git a/uploader/background_jobs.py b/uploader/background_jobs.py index 09ea0c0..dc9f837 100644 --- a/uploader/background_jobs.py +++ b/uploader/background_jobs.py @@ -1,11 +1,10 @@ +"""Generic views and utilities to handle background jobs.""" import uuid -import logging import importlib from typing import Callable from functools import partial from flask import ( - request, url_for, redirect, Response, @@ -29,8 +28,11 @@ def __default_error_handler__(job: dict) -> Response: def register_handlers( job_type: str, success_handler: HandlerType, + # pylint: disable=[redefined-outer-name] error_handler: HandlerType = __default_error_handler__ + # pylint: disable=[redefined-outer-name] ) -> str: + """Register success and error handlers for each job type.""" if not bool(app.config.get("background-jobs")): app.config["background-jobs"] = {} @@ -49,19 +51,19 @@ def register_job_handlers(job: str): _parts = absolute_function_path.split(".") app.logger.debug("THE PARTS ARE: %s", _parts) assert len(_parts) > 1, f"Invalid path: {absolute_function_path}" - function = _parts[-1] module = importlib.import_module(f".{_parts[-2]}", package=".".join(_parts[0:-2])) return getattr(module, _parts[-1]) metadata = job["metadata"] if metadata["success_handler"]: - success_handler = __load_handler__(metadata["success_handler"]) + _success_handler = __load_handler__(metadata["success_handler"]) try: - error_handler = __load_handler__(metadata["error_handler"]) - except Exception as _exc: - error_handler = __default_error_handler__ - register_handlers(metadata["job-type"], success_handler, error_handler) + _error_handler = __load_handler__(metadata["error_handler"]) + except Exception as _exc:# pylint: disable=[broad-exception-caught] + _error_handler = __default_error_handler__ + register_handlers( + metadata["job-type"], _success_handler, _error_handler) def handler(job: dict, handler_type: str) -> HandlerType: @@ -74,7 +76,7 @@ def handler(job: dict, handler_type: str) -> HandlerType: ).get(handler_type) if bool(_handler): return _handler(job) - raise Exception( + raise Exception(# pylint: disable=[broad-exception-raised] f"No '{handler_type}' handler registered for job type: {_job_type}") @@ -98,8 +100,8 @@ def job_status(job_id: uuid.UUID): if status == "completed": return success_handler(job) - return render_template(f"jobs/job-status.html", job=job) - except JobNotFound as jnf: + return render_template("jobs/job-status.html", job=job) + except JobNotFound as _jnf: return render_template( "jobs/job-not-found.html", job_id=job_id) @@ -108,9 +110,10 @@ def job_status(job_id: uuid.UUID): @background_jobs_bp.route("/error/<uuid:job_id>") @require_login def job_error(job_id: uuid.UUID): + """Handle job errors in a generic manner.""" with sqlite3.connection(app.config["ASYNCHRONOUS_JOBS_SQLITE_DB"]) as conn: try: job = jobs.job(conn, job_id, fulldetails=True) return render_template("jobs/job-error.html", job=job) - except JobNotFound as jnf: + except JobNotFound as _jnf: return render_template("jobs/job-not-found.html", job_id=job_id) diff --git a/uploader/jobs.py b/uploader/jobs.py index e86ee05..5968c03 100644 --- a/uploader/jobs.py +++ b/uploader/jobs.py @@ -41,7 +41,8 @@ def error_filename(jobid, error_dir): "Compute the path of the file where errors will be dumped." return f"{error_dir}/job_{jobid}.error" -def initialise_job(# pylint: disable=[too-many-arguments] +def initialise_job( + # pylint: disable=[too-many-arguments, too-many-positional-arguments] rconn: Redis, rprefix: str, jobid: str, command: list, job_type: str, ttl_seconds: int = 86400, extra_meta: Optional[dict] = None) -> dict: "Initialise a job 'object' and put in on redis" @@ -54,7 +55,8 @@ def initialise_job(# pylint: disable=[too-many-arguments] name=job_key(rprefix, jobid), time=timedelta(seconds=ttl_seconds)) return the_job -def build_file_verification_job(#pylint: disable=[too-many-arguments] +def build_file_verification_job( + #pylint: disable=[too-many-arguments, too-many-positional-arguments] redis_conn: Redis, dburi: str, redisuri: str, @@ -77,7 +79,8 @@ def build_file_verification_job(#pylint: disable=[too-many-arguments] "filename": os.path.basename(filepath), "percent": 0 }) -def data_insertion_job(# pylint: disable=[too-many-arguments] +def data_insertion_job( + # pylint: disable=[too-many-arguments, too-many-positional-arguments] redis_conn: Redis, filepath: str, filetype: str, totallines: int, speciesid: int, platformid: int, datasetid: int, databaseuri: str, redisuri: str, ttl_seconds: int) -> dict: diff --git a/uploader/monadic_requests.py b/uploader/monadic_requests.py index f1f5c77..eda42d0 100644 --- a/uploader/monadic_requests.py +++ b/uploader/monadic_requests.py @@ -59,6 +59,11 @@ def get(url, params=None, **kwargs) -> Either: :rtype: pymonad.either.Either """ + timeout = kwargs.get("timeout") + kwargs = {key: val for key,val in kwargs.items() if key != "timeout"} + if timeout is None: + timeout = (9.13, 20) + try: resp = requests.get(url, params=params, **kwargs) if resp.status_code in SUCCESS_CODES: @@ -76,6 +81,11 @@ def post(url, data=None, json=None, **kwargs) -> Either: :rtype: pymonad.either.Either """ + timeout = kwargs.get("timeout") + kwargs = {key: val for key,val in kwargs.items() if key != "timeout"} + if timeout is None: + timeout = (9.13, 20) + try: resp = requests.post(url, data=data, json=json, **kwargs) if resp.status_code in SUCCESS_CODES: @@ -95,10 +105,10 @@ def make_either_error_handler(msg): try: _data = error.json() except Exception as _exc: - raise Exception(error.content) from _exc - raise Exception(_data) + raise Exception(error.content) from _exc# pylint: disable=[broad-exception-raised] + raise Exception(_data)# pylint: disable=[broad-exception-raised] app.logger.debug("\n\n%s\n\n", msg) - raise Exception(error) + raise Exception(error)# pylint: disable=[broad-exception-raised] return __fail__ diff --git a/uploader/platforms/models.py b/uploader/platforms/models.py index a859371..0dd9368 100644 --- a/uploader/platforms/models.py +++ b/uploader/platforms/models.py @@ -56,7 +56,8 @@ def platform_by_species_and_id( return None -def save_new_platform(# pylint: disable=[too-many-arguments] +def save_new_platform( + # pylint: disable=[too-many-arguments, too-many-positional-arguments] cursor: Cursor, species_id: int, geo_platform: str, diff --git a/uploader/route_utils.py b/uploader/route_utils.py index 18eadda..ce718fb 100644 --- a/uploader/route_utils.py +++ b/uploader/route_utils.py @@ -6,7 +6,8 @@ from gn_libs.mysqldb import database_connection from uploader.population.models import (populations_by_species, population_by_species_and_id) -def generic_select_population(# pylint: disable=[too-many-arguments] +def generic_select_population( + # pylint: disable=[too-many-arguments, too-many-positional-arguments] species: dict, template: str, population_id: str, |