From c8ca5b44b83b4d1d2cfa491d998818a16ccf40c3 Mon Sep 17 00:00:00 2001 From: Cody Baker Date: Wed, 22 May 2024 02:25:33 -0400 Subject: [PATCH 1/6] adding docstrings and standardizing errors --- pyflask/apis/data.py | 16 +- pyflask/apis/neuroconv.py | 13 +- pyflask/app.py | 137 ++++++++++-------- pyflask/errorHandlers/__init__.py | 1 - .../errorHandlers/notBadRequestException.py | 5 - pyflask/tests/conftest.py | 4 +- pyflask/utils/__init__.py | 3 + pyflask/utils/_catch_flask_errors.py | 27 ++++ 8 files changed, 125 insertions(+), 81 deletions(-) delete mode 100644 pyflask/errorHandlers/__init__.py delete mode 100644 pyflask/errorHandlers/notBadRequestException.py create mode 100644 pyflask/utils/__init__.py create mode 100644 pyflask/utils/_catch_flask_errors.py diff --git a/pyflask/apis/data.py b/pyflask/apis/data.py index de6dfdc6d..688ea0e2e 100644 --- a/pyflask/apis/data.py +++ b/pyflask/apis/data.py @@ -5,6 +5,7 @@ from errorHandlers import notBadRequestException from flask_restx import Namespace, Resource, reqparse from manageNeuroconv import generate_dataset, generate_test_data +from utils import catch_exception_and_abort, server_error_responses data_api = Namespace("data", description="API route for dataset generation in the NWB GUIDE.") @@ -22,15 +23,14 @@ def exception_handler(error): @data_api.route("/generate") @data_api.expect(generate_test_data_parser) class GeneratetestData(Resource): - @data_api.doc(responses={200: "Success", 400: "Bad Request", 500: "Internal server error"}) + @data_api.doc( + description="Generate example ecephys data using SpikeInterface.", + responses=server_error_responses(codes=[200, 500]), + ) + @catch_exception_and_abort(api=data_api, code=500) def post(self): - try: - arguments = generate_test_data_parser.parse_args() - generate_test_data(output_path=arguments["output_path"]) - except Exception as exception: - if notBadRequestException(exception): - data_api.abort(500, str(exception)) - raise exception + arguments = generate_test_data_parser.parse_args() + generate_test_data(output_path=arguments["output_path"]) generate_test_dataset_parser = reqparse.RequestParser() diff --git a/pyflask/apis/neuroconv.py b/pyflask/apis/neuroconv.py index 015cb61e8..e2b9a9257 100644 --- a/pyflask/apis/neuroconv.py +++ b/pyflask/apis/neuroconv.py @@ -1,6 +1,7 @@ """API endpoint definitions for interacting with NeuroConv.""" import traceback +from typing import Dict from errorHandlers import notBadRequestException from flask import Response, request @@ -25,16 +26,18 @@ ) from manageNeuroconv.info import announcer -neuroconv_api = Namespace("neuroconv", description="Neuroconv neuroconv_api for the NWB GUIDE.") +neuroconv_api = Namespace(name="neuroconv", description="Neuroconv neuroconv_api for the NWB GUIDE.") parser = reqparse.RequestParser() -parser.add_argument("interfaces", type=str, action="split", help="Interfaces cannot be converted") +parser.add_argument("interfaces", type=str, action="split", help="Interfaces cannot be converted.") @neuroconv_api.errorhandler(Exception) -def exception_handler(error): - exceptiondata = traceback.format_exception(type(error), error, error.__traceback__) - return {"message": exceptiondata[-1], "traceback": "".join(exceptiondata)} +def exception_handler(error: Exception) -> Dict[str, str]: + full_traceback = traceback.format_exception(type(error), error, error.__traceback__) + message = full_traceback[-1] + remaining_traceback = "".join(full_traceback[:-1]) + return {"message": message, "traceback": remaining_traceback} @neuroconv_api.route("/") diff --git a/pyflask/app.py b/pyflask/app.py index a75fb88bb..3dcdbe886 100644 --- a/pyflask/app.py +++ b/pyflask/app.py @@ -12,14 +12,12 @@ from signal import SIGINT from urllib.parse import unquote -from errorHandlers import notBadRequestException - # https://stackoverflow.com/questions/32672596/pyinstaller-loads-script-multiple-times#comment103216434_32677108 multiprocessing.freeze_support() from apis import data_api, neuroconv_api, startup_api -from flask import Flask, request, send_file, send_from_directory +from flask import Flask, Response, request, send_file, send_from_directory from flask_cors import CORS from flask_restx import Api, Resource from manageNeuroconv.info import ( @@ -28,12 +26,13 @@ STUB_SAVE_FOLDER_PATH, resource_path, ) +from utils import catch_exception_and_abort, server_error_responses -app = Flask(__name__) +flask_app = Flask(__name__) # Always enable CORS to allow distinct processes to handle frontend vs. backend -CORS(app) -app.config["CORS_HEADERS"] = "Content-Type" +CORS(flask_app) +flask_app.config["CORS_HEADERS"] = "Content-Type" # Create logger configuration LOG_FOLDER = Path(GUIDE_ROOT_FOLDER, "logs") @@ -54,46 +53,85 @@ api.add_namespace(startup_api) api.add_namespace(neuroconv_api) api.add_namespace(data_api) -api.init_app(app) +api.init_app(flask_app) + +# 'nwbfile_registry' is a global list that keeps track of all NWB files that have been registered with the server +nwbfile_registry = [] + + +# TODO: is there any advantage to using the api.route instead of app for resources added in this file? +@api.route(rule="/log") +@api.doc( + description="Any exception that occurs on the Flask server will save a full traceback to a log file on disk.", + responses=server_error_responses(codes=[200, 400, 500]), +) +class Log(Resource): + @api.doc(description="Nicely format the exception and the payload that caused it.", responses=all_error_responses) + @catch_exception_and_abort(api=api, code=500) + def post(self): + payload = api.payload + type = payload["type"] + header = payload["header"] + inputs = payload["inputs"] + exception_traceback = payload["traceback"] -registered = {} + message = f"{header}\n{'-'*len(header)}\n\n{json.dumps(inputs, indent=2)}\n\n{exception_traceback}\n" + selected_logger = getattr(api.logger, type) + selected_logger(message) -@app.route("/files") +@flask_app.route(rule="/files") +@api.doc( + description="Get a list of all files that have been nwbfile_registry with the server.", + responses=server_error_responses(codes=[200, 400, 500]), +) +@catch_exception_and_abort(api=api, code=500) def get_all_files(): - return list(registered.keys()) + return list(nwbfile_registry.keys()) -@app.route("/files/", methods=["GET", "POST"]) -def handle_file_request(path): - if request.method == "GET": - if registered[path]: - path = unquote(path) - if not isabs(path): - path = f"/{path}" - return send_file(path) - else: - app.abort(404, "Resource is not accessible.") +@flask_app.route(rule="/files/", methods=["GET", "POST"]) +@api.doc( + description="Handle adding and fetching NWB files from the global file registry.", + responses=server_error_responses(codes=[200, 400, 404, 500]), +) +def handle_file_request(file_path: str) -> Union[str, Response, None]: + if ".nwb" not in file_path: + code = 400 + base_message = server_error_responses(codes=[code])[code] + message = f"{base_message}: Path does not point to an NWB file." + flask_app.abort(code=code, message=message) + return + + if request.method == "GET" and file_path not in nwbfile_registry: + code = 404 + base_message = server_error_responses(codes=[code])[code] + message = f"{base_message}: Path does not point to an NWB file." + flask_app.abort(code=code, message=message) + return - else: - if ".nwb" in path: - registered[path] = True - return request.base_url - else: - app.abort(400, str("Path does not point to an NWB file.")) + if request.method == "GET": + parsed_file_path = unquote(file_path) + is_file_relative = not isabs(parsed_file_path) + if is_file_relative: + parsed_file_path = f"/{parsed_file_path}" + return send_file(path_or_file=parsed_file_path) + elif request.method == "POST": + nwbfile_registry.append(file_path) + return request.base_url -@app.route("/conversions/") +@flask_app.route("/conversions/") def send_conversions(path): return send_from_directory(CONVERSION_SAVE_FOLDER_PATH, path) -@app.route("/preview/") +@flask_app.route("/preview/") def send_preview(path): return send_from_directory(STUB_SAVE_FOLDER_PATH, path) -@app.route("/cpus") +@flask_app.route("/cpus") def get_cpu_count(): from psutil import cpu_count @@ -103,45 +141,24 @@ def get_cpu_count(): return dict(physical=physical, logical=logical) -@app.route("/get-recommended-species") +@flask_app.route("/get-recommended-species") def get_species(): from dandi.metadata.util import species_map return species_map -@api.route("/log") -class Log(Resource): - @api.doc(responses={200: "Success", 400: "Bad Request", 500: "Internal server error"}) - def post(self): - try: - - payload = api.payload - type = payload["type"] - header = payload["header"] - inputs = payload["inputs"] - traceback = payload["traceback"] - - message = f"{header}\n{'-'*len(header)}\n\n{json.dumps(inputs, indent=2)}\n\n{traceback}\n" - selected_logger = getattr(api.logger, type) - selected_logger(message) - - except Exception as exception: - if notBadRequestException(exception): - api.abort(500, str(exception)) - - @api.route("/server_shutdown", endpoint="shutdown") class Shutdown(Resource): - def get(self): - func = request.environ.get("werkzeug.server.shutdown") - api.logger.info("Shutting down server") + def get(self) -> None: + werkzeug_shutdown_function = request.environ.get("werkzeug.server.shutdown") + api.logger.info("Shutting down server...") - if func is None: + if werkzeug_shutdown_function is None: kill(getpid(), SIGINT) return - func() + werkzeug_shutdown_function() if __name__ == "__main__": @@ -156,13 +173,13 @@ def get(self): ) log_handler.setFormatter(log_formatter) - app.logger.addHandler(log_handler) - app.logger.setLevel(DEBUG) + flask_app.logger.addHandler(log_handler) + flask_app.logger.setLevel(DEBUG) - app.logger.info(f"Logging to {LOG_FILE_PATH}") + flask_app.logger.info(f"Logging to {LOG_FILE_PATH}") # Run the server api.logger.info(f"Starting server on port {port}") - app.run(host="127.0.0.1", port=port) + flask_app.run(host="127.0.0.1", port=port) else: raise Exception("No port provided for the NWB GUIDE backend.") diff --git a/pyflask/errorHandlers/__init__.py b/pyflask/errorHandlers/__init__.py deleted file mode 100644 index b88aad104..000000000 --- a/pyflask/errorHandlers/__init__.py +++ /dev/null @@ -1 +0,0 @@ -from .notBadRequestException import notBadRequestException diff --git a/pyflask/errorHandlers/notBadRequestException.py b/pyflask/errorHandlers/notBadRequestException.py deleted file mode 100644 index c0f002952..000000000 --- a/pyflask/errorHandlers/notBadRequestException.py +++ /dev/null @@ -1,5 +0,0 @@ -def notBadRequestException(exception): - """ - Check if the exception is a generic exception. - """ - return type(exception).__name__ not in ["BadRequest", "Forbidden", "Unauthorized"] diff --git a/pyflask/tests/conftest.py b/pyflask/tests/conftest.py index ec72c792b..0a19565fa 100644 --- a/pyflask/tests/conftest.py +++ b/pyflask/tests/conftest.py @@ -3,7 +3,7 @@ def pytest_addoption(parser): - parser.addoption("--target", action="store", help="Run the executable instead of the standard Flask app") + parser.addoption("--target", action="store", help="Run the executable instead of the standard Flask flask_app") @pytest.fixture(scope="session") @@ -12,7 +12,7 @@ def client(request): if target: return target else: - app = flask.app + app = flask.flask_app app.config.update( { "TESTING": True, diff --git a/pyflask/utils/__init__.py b/pyflask/utils/__init__.py new file mode 100644 index 000000000..19b2edfa5 --- /dev/null +++ b/pyflask/utils/__init__.py @@ -0,0 +1,3 @@ +from ._catch_flask_errors import catch_exception_and_abort, server_error_responses + +__all__ = ["catch_exception_and_abort", "server_error_responses"] diff --git a/pyflask/utils/_catch_flask_errors.py b/pyflask/utils/_catch_flask_errors.py new file mode 100644 index 000000000..be5b9101d --- /dev/null +++ b/pyflask/utils/_catch_flask_errors.py @@ -0,0 +1,27 @@ +import contextlib + +import flask_restx +from tpying import List, Union + + +def server_error_responses(*, codes: List[str]) -> Dict[int, str]: + all_server_error_responses = { + 200: "Success", + 400: "Bad request", + 404: "Resource is not accessible.", + 500: ("Internalerver error"), + } + + selected_responses = {code: all_server_error_responses[code] for code in codes} + return selected_responses + + +@contextlib.contextmanager +def catch_exception_and_abort(*, api: Union[flask_restx.Api, flask_restx.Namespace], code: int) -> None: + try: + yield + except Exception as exception: + exception_type = type(exception) + exception_message = str(exception) + api.abort(code=code, message=f"{exception_type}: {exception_message}") + raise exception From 60e3409a2aec95db587d033c21c36653eb24b061 Mon Sep 17 00:00:00 2001 From: Cody Baker <51133164+CodyCBakerPhD@users.noreply.github.com> Date: Wed, 22 May 2024 11:01:19 -0400 Subject: [PATCH 2/6] make specific workflow for dev server tests --- .github/workflows/flask_dev_tests.yml | 92 +++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 .github/workflows/flask_dev_tests.yml diff --git a/.github/workflows/flask_dev_tests.yml b/.github/workflows/flask_dev_tests.yml new file mode 100644 index 000000000..96b4c084c --- /dev/null +++ b/.github/workflows/flask_dev_tests.yml @@ -0,0 +1,92 @@ +name: Flask Dev Tests +on: + schedule: + - cron: "0 17 * * *" # Daily at 1pm EST + pull_request: + workflow_dispatch: + +# Cancel previous workflows on the same pull request +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +env: + CACHE_NUMBER: 1 # increase to reset cache manually + +jobs: + testing: + name: ${{ matrix.os }} # Will read on the dashboard as 'Flask Dev Tests / {os}' + runs-on: ${{ matrix.os }} + defaults: + run: + shell: bash -l {0} + + strategy: + fail-fast: false + matrix: + include: + # current linux installation instructions use dev mode instead of distributable + # - python-version: "3.9" + # os: ubuntu-latest + # label: environments/environment-Linux.yml + # prefix: /usr/share/miniconda3/envs/nwb-guide + + - python-version: "3.9" + os: macos-latest # Mac arm64 runner + label: environments/environment-MAC-apple-silicon.yml + prefix: /Users/runner/miniconda3/envs/nwb-guide + + - python-version: "3.9" + os: macos-13 # Mac x64 runner + label: environments/environment-MAC-intel.yml + prefix: /Users/runner/miniconda3/envs/nwb-guide + + - python-version: "3.9" + os: windows-latest + label: environments/environment-Windows.yml + prefix: C:\Miniconda3\envs\nwb-guide + + steps: + - uses: actions/checkout@v4 + - run: git fetch --prune --unshallow --tags + + - name: Printout architecture + run: uname -m + + # see https://github.com/conda-incubator/setup-miniconda#caching-environments + - name: Setup Mambaforge + uses: conda-incubator/setup-miniconda@v3 + with: + miniforge-variant: Mambaforge + miniforge-version: latest + activate-environment: nwb-guide + use-mamba: true + + - name: Set cache date + id: get-date + run: echo "today=$(/bin/date -u '+%Y%m%d')" >> $GITHUB_OUTPUT + shell: bash + + - name: Cache Mamba env + uses: actions/cache@v4 + with: + path: ${{ env.CONDA }}/envs + key: + conda-${{ runner.os }}-${{ runner.arch }}-${{steps.get-date.outputs.today }}-${{ hashFiles(matrix.label) }}-${{ env.CACHE_NUMBER }} + env: + CACHE_NUMBER: ${{ env.CACHE_NUMBER }} + id: cache + + - if: steps.cache.outputs.cache-hit != 'true' + name: Update environment + run: mamba env update -f ${{ matrix.label }} + + - name: Setup Node.js 20 + uses: actions/setup-node@v4 + with: + node-version: "20" + + - run: npm ci --verbose + + - name: Run Python tests on local dev mode + run: npm run test:server From 98641b0e6e17311e83126efe483a1f5b05e1897e Mon Sep 17 00:00:00 2001 From: Cody Baker <51133164+CodyCBakerPhD@users.noreply.github.com> Date: Wed, 22 May 2024 11:04:07 -0400 Subject: [PATCH 3/6] adjust coverage name --- .../workflows/{testing.yml => app_and_flask_coverage.yml} | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) rename .github/workflows/{testing.yml => app_and_flask_coverage.yml} (95%) diff --git a/.github/workflows/testing.yml b/.github/workflows/app_and_flask_coverage.yml similarity index 95% rename from .github/workflows/testing.yml rename to .github/workflows/app_and_flask_coverage.yml index 4dbace72d..8906c01bb 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/app_and_flask_coverage.yml @@ -1,8 +1,9 @@ -name: Dev Tests +name: App and Flask Coverage on: schedule: - cron: "0 16 * * *" # Daily at noon EST pull_request: + workflow_dispatch: concurrency: # Cancel previous workflows on the same pull request group: ${{ github.workflow }}-${{ github.ref }} @@ -13,7 +14,7 @@ env: jobs: testing: - name: ${{ matrix.os }} # Will read on the dashboard as 'Dev Tests / {os}' + name: ${{ matrix.os }} # Will read on the dashboard as 'App and Flask Coverage / {os}' runs-on: ${{ matrix.os }} defaults: run: From 13223f91858dd67b7c8b2de1913f966d2c35984d Mon Sep 17 00:00:00 2001 From: Cody Baker Date: Wed, 22 May 2024 11:27:44 -0400 Subject: [PATCH 4/6] cleanup related to error handling --- pyflask/apis/data.py | 4 +-- pyflask/apis/neuroconv.py | 52 +++++++++++++-------------------------- pyflask/app.py | 1 + 3 files changed, 19 insertions(+), 38 deletions(-) diff --git a/pyflask/apis/data.py b/pyflask/apis/data.py index 688ea0e2e..9bf43c540 100644 --- a/pyflask/apis/data.py +++ b/pyflask/apis/data.py @@ -2,7 +2,6 @@ import traceback -from errorHandlers import notBadRequestException from flask_restx import Namespace, Resource, reqparse from manageNeuroconv import generate_dataset, generate_test_data from utils import catch_exception_and_abort, server_error_responses @@ -48,5 +47,4 @@ def post(self): return generate_dataset(input_path=arguments["input_path"], output_path=arguments["output_path"]) except Exception as exception: - if notBadRequestException(exception): - data_api.abort(500, str(exception)) + data_api.abort(500, str(exception)) diff --git a/pyflask/apis/neuroconv.py b/pyflask/apis/neuroconv.py index e2b9a9257..98d85798b 100644 --- a/pyflask/apis/neuroconv.py +++ b/pyflask/apis/neuroconv.py @@ -3,7 +3,6 @@ import traceback from typing import Dict -from errorHandlers import notBadRequestException from flask import Response, request from flask_restx import Namespace, Resource, reqparse from manageNeuroconv import ( @@ -53,8 +52,7 @@ def get(self): **get_all_converter_info(), } except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) raise exception @@ -65,8 +63,7 @@ def post(self): try: return get_source_schema(neuroconv_api.payload) except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) @neuroconv_api.route("/locate") @@ -76,8 +73,7 @@ def post(self): try: return locate_data(neuroconv_api.payload) except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) @neuroconv_api.route("/locate/autocomplete") @@ -87,8 +83,7 @@ def post(self): try: return autocomplete_format_string(neuroconv_api.payload) except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) @neuroconv_api.route("/metadata") @@ -100,8 +95,7 @@ def post(self): neuroconv_api.payload.get("source_data"), neuroconv_api.payload.get("interfaces") ) except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) @neuroconv_api.route("/convert") @@ -112,8 +106,7 @@ def post(self): return convert_to_nwb(neuroconv_api.payload) except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) @neuroconv_api.route("/alignment") @@ -124,8 +117,7 @@ def post(self): return get_interface_alignment(neuroconv_api.payload) except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) validate_parser = neuroconv_api.parser() @@ -146,8 +138,7 @@ def post(self): return validate_metadata(args.get("parent"), args.get("function_name")) except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) @neuroconv_api.route("/upload/project") @@ -166,8 +157,7 @@ def post(self): return upload_project_to_dandi(**upload_options) except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) @neuroconv_api.route("/upload/folder") @@ -186,8 +176,7 @@ def post(self): return upload_folder_to_dandi(**upload_options) except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) @neuroconv_api.route("/upload") @@ -209,8 +198,7 @@ def post(self): return upload_multiple_filesystem_objects_to_dandi(**neuroconv_api.payload) except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) @neuroconv_api.route("/inspect_file") @@ -220,8 +208,7 @@ def post(self): try: return inspect_nwb_file(neuroconv_api.payload) except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) @neuroconv_api.route("/inspect_folder") @@ -233,8 +220,7 @@ def post(self): return inspect_nwb_folder(url, neuroconv_api.payload) except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) @neuroconv_api.route("/announce") @@ -246,8 +232,7 @@ def post(self): announcer.announce(data) return True except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) @neuroconv_api.route("/inspect") @@ -274,8 +259,7 @@ def post(self): return inspect_multiple_filesystem_objects(url, paths, **kwargs) except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) @neuroconv_api.route("/html") @@ -290,8 +274,7 @@ def post(self): return html except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) # Create an events endpoint @@ -304,5 +287,4 @@ def get(self): return Response(listen_to_neuroconv_events(), mimetype="text/event-stream") except Exception as exception: - if notBadRequestException(exception): - neuroconv_api.abort(500, str(exception)) + neuroconv_api.abort(500, str(exception)) diff --git a/pyflask/app.py b/pyflask/app.py index 3dcdbe886..3dd8eef99 100644 --- a/pyflask/app.py +++ b/pyflask/app.py @@ -96,6 +96,7 @@ def get_all_files(): responses=server_error_responses(codes=[200, 400, 404, 500]), ) def handle_file_request(file_path: str) -> Union[str, Response, None]: + """Used by the PreviewPage to serve the URL to Neurosift.""" if ".nwb" not in file_path: code = 400 base_message = server_error_responses(codes=[code])[code] From 66e7f838a6c1928e86acee7f7bc25070f5b0ed41 Mon Sep 17 00:00:00 2001 From: Garrett Michael Flynn Date: Wed, 22 May 2024 09:12:04 -0700 Subject: [PATCH 5/6] Add fixes and remove extraneous Neurosift-related endpoints --- pyflask/apis/startup.py | 4 +- pyflask/app.py | 41 ++++++------------- pyflask/utils/_catch_flask_errors.py | 2 +- .../src/stories/pages/preview/PreviewPage.js | 5 +++ .../src/stories/preview/NWBFilePreview.js | 33 ++++++++++----- src/renderer/src/stories/preview/Neurosift.js | 6 --- 6 files changed, 42 insertions(+), 49 deletions(-) diff --git a/pyflask/apis/startup.py b/pyflask/apis/startup.py index 63be4fd0a..823dbee00 100644 --- a/pyflask/apis/startup.py +++ b/pyflask/apis/startup.py @@ -1,6 +1,5 @@ """API endpoint definitions for startup operations.""" -from errorHandlers import notBadRequestException from flask_restx import Namespace, Resource startup_api = Namespace("startup", description="API for startup commands related to the NWB GUIDE.") @@ -40,6 +39,5 @@ def get(self): return True except Exception as exception: - if notBadRequestException(exception=exception): - startup_api.abort(500, str(exception)) + startup_api.abort(500, str(exception)) raise exception diff --git a/pyflask/app.py b/pyflask/app.py index 3dd8eef99..8ee3d7689 100644 --- a/pyflask/app.py +++ b/pyflask/app.py @@ -12,6 +12,8 @@ from signal import SIGINT from urllib.parse import unquote +from typing import Union + # https://stackoverflow.com/questions/32672596/pyinstaller-loads-script-multiple-times#comment103216434_32677108 multiprocessing.freeze_support() @@ -66,7 +68,7 @@ responses=server_error_responses(codes=[200, 400, 500]), ) class Log(Resource): - @api.doc(description="Nicely format the exception and the payload that caused it.", responses=all_error_responses) + @api.doc(description="Nicely format the exception and the payload that caused it.", responses=server_error_responses(codes=[200, 400, 404, 500])) @catch_exception_and_abort(api=api, code=500) def post(self): payload = api.payload @@ -79,18 +81,8 @@ def post(self): selected_logger = getattr(api.logger, type) selected_logger(message) - -@flask_app.route(rule="/files") -@api.doc( - description="Get a list of all files that have been nwbfile_registry with the server.", - responses=server_error_responses(codes=[200, 400, 500]), -) -@catch_exception_and_abort(api=api, code=500) -def get_all_files(): - return list(nwbfile_registry.keys()) - - -@flask_app.route(rule="/files/", methods=["GET", "POST"]) +# Used for the standalone preview page +@flask_app.route(rule="/files/", methods=["GET", "POST"]) @api.doc( description="Handle adding and fetching NWB files from the global file registry.", responses=server_error_responses(codes=[200, 400, 404, 500]), @@ -101,36 +93,27 @@ def handle_file_request(file_path: str) -> Union[str, Response, None]: code = 400 base_message = server_error_responses(codes=[code])[code] message = f"{base_message}: Path does not point to an NWB file." - flask_app.abort(code=code, message=message) + api.abort(code=code, message=message) return if request.method == "GET" and file_path not in nwbfile_registry: code = 404 base_message = server_error_responses(codes=[code])[code] message = f"{base_message}: Path does not point to an NWB file." - flask_app.abort(code=code, message=message) + api.abort(code=code, message=message) return if request.method == "GET": - parsed_file_path = unquote(file_path) - is_file_relative = not isabs(parsed_file_path) + parsed_file_path = unquote(file_path) # Decode any URL encoding applied to the file path + is_file_relative = not isabs(parsed_file_path) # Check if the file path is relative if is_file_relative: parsed_file_path = f"/{parsed_file_path}" return send_file(path_or_file=parsed_file_path) + + # Register access to the provided file path elif request.method == "POST": nwbfile_registry.append(file_path) - return request.base_url - - -@flask_app.route("/conversions/") -def send_conversions(path): - return send_from_directory(CONVERSION_SAVE_FOLDER_PATH, path) - - -@flask_app.route("/preview/") -def send_preview(path): - return send_from_directory(STUB_SAVE_FOLDER_PATH, path) - + return request.base_url # Return the URL of the newly added file @flask_app.route("/cpus") def get_cpu_count(): diff --git a/pyflask/utils/_catch_flask_errors.py b/pyflask/utils/_catch_flask_errors.py index be5b9101d..a84e50955 100644 --- a/pyflask/utils/_catch_flask_errors.py +++ b/pyflask/utils/_catch_flask_errors.py @@ -1,7 +1,7 @@ import contextlib import flask_restx -from tpying import List, Union +from typing import List, Union, Dict def server_error_responses(*, codes: List[str]) -> Dict[int, str]: diff --git a/src/renderer/src/stories/pages/preview/PreviewPage.js b/src/renderer/src/stories/pages/preview/PreviewPage.js index a8e6a320a..d14f3c61a 100644 --- a/src/renderer/src/stories/pages/preview/PreviewPage.js +++ b/src/renderer/src/stories/pages/preview/PreviewPage.js @@ -18,8 +18,13 @@ export class PreviewPage extends Page { updatePath = async (path) => { if (path) { + + // Enable access to the explicit file path const result = await fetch(`${baseUrl}/files/${path}`, { method: "POST" }).then((res) => res.text()); + + // Set Neurosift to access the returned URL if (result) this.neurosift.url = result; + } else this.neurosift.url = undefined; }; diff --git a/src/renderer/src/stories/preview/NWBFilePreview.js b/src/renderer/src/stories/preview/NWBFilePreview.js index 4bc8b7b71..a7c3fba8d 100644 --- a/src/renderer/src/stories/preview/NWBFilePreview.js +++ b/src/renderer/src/stories/preview/NWBFilePreview.js @@ -1,12 +1,13 @@ import { LitElement, css, html } from "lit"; import { InspectorList } from "./inspector/InspectorList"; -import { Neurosift, getURLFromFilePath } from "./Neurosift"; +import { Neurosift } from "./Neurosift"; import { unsafeHTML } from "lit/directives/unsafe-html.js"; import { run } from "../pages/guided-mode/options/utils"; import { until } from "lit/directives/until.js"; import { InstanceManager } from "../InstanceManager"; import { path } from "../../electron"; import { FullScreenToggle } from "../FullScreenToggle"; +import { baseUrl } from "../../server/globals"; export function getSharedPath(array) { array = array.map((str) => str.replace(/\\/g, "/")); // Convert to Mac-style path @@ -55,15 +56,27 @@ class NWBPreviewInstance extends LitElement { render() { const isOnline = navigator.onLine; - return isOnline - ? new Neurosift({ url: getURLFromFilePath(this.file, this.project), fullscreen: false }) - : until( - (async () => { - const htmlRep = await run("html", { nwbfile_path: this.file }, { swal: false }); - return unsafeHTML(htmlRep); - })(), - html`Loading HTML representation...` - ); + if (!isOnline) return until( + (async () => { + const htmlRep = await run("html", { nwbfile_path: this.file }, { swal: false }); + return unsafeHTML(htmlRep); + })(), + html`Loading HTML representation...` + ); + + const neurosift = new Neurosift({ fullscreen: false }) + + // Enable access to the explicit file path + fetch(`${baseUrl}/files/${this.file}`, { method: "POST" }) + .then((res) => res.text()) + .then((result) => { + + // Set Neurosift to access the returned URL + if (result) neurosift.url = result; + }) + + + return neurosift } } diff --git a/src/renderer/src/stories/preview/Neurosift.js b/src/renderer/src/stories/preview/Neurosift.js index 8def8c8dd..e60b2e467 100644 --- a/src/renderer/src/stories/preview/Neurosift.js +++ b/src/renderer/src/stories/preview/Neurosift.js @@ -2,12 +2,6 @@ import { LitElement, css, html } from "lit"; import { Loader } from "../Loader"; import { FullScreenToggle } from "../FullScreenToggle"; -import { baseUrl } from "../../server/globals"; - -export function getURLFromFilePath(file, projectName) { - const regexp = new RegExp(`.+(${projectName}.+)`); - return `${baseUrl}/preview/${file.match(regexp)[1]}`; -} export class Neurosift extends LitElement { static get styles() { From b3b1d24f027e7a8d92a4a8df8293efb4a9ce109d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 22 May 2024 16:12:24 +0000 Subject: [PATCH 6/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pyflask/app.py | 18 ++++++----- pyflask/utils/_catch_flask_errors.py | 2 +- .../src/stories/pages/preview/PreviewPage.js | 2 -- .../src/stories/preview/NWBFilePreview.js | 31 +++++++++---------- 4 files changed, 27 insertions(+), 26 deletions(-) diff --git a/pyflask/app.py b/pyflask/app.py index 8ee3d7689..c254cc889 100644 --- a/pyflask/app.py +++ b/pyflask/app.py @@ -10,9 +10,8 @@ from os.path import isabs from pathlib import Path from signal import SIGINT -from urllib.parse import unquote - from typing import Union +from urllib.parse import unquote # https://stackoverflow.com/questions/32672596/pyinstaller-loads-script-multiple-times#comment103216434_32677108 multiprocessing.freeze_support() @@ -68,7 +67,10 @@ responses=server_error_responses(codes=[200, 400, 500]), ) class Log(Resource): - @api.doc(description="Nicely format the exception and the payload that caused it.", responses=server_error_responses(codes=[200, 400, 404, 500])) + @api.doc( + description="Nicely format the exception and the payload that caused it.", + responses=server_error_responses(codes=[200, 400, 404, 500]), + ) @catch_exception_and_abort(api=api, code=500) def post(self): payload = api.payload @@ -81,6 +83,7 @@ def post(self): selected_logger = getattr(api.logger, type) selected_logger(message) + # Used for the standalone preview page @flask_app.route(rule="/files/", methods=["GET", "POST"]) @api.doc( @@ -104,16 +107,17 @@ def handle_file_request(file_path: str) -> Union[str, Response, None]: return if request.method == "GET": - parsed_file_path = unquote(file_path) # Decode any URL encoding applied to the file path - is_file_relative = not isabs(parsed_file_path) # Check if the file path is relative + parsed_file_path = unquote(file_path) # Decode any URL encoding applied to the file path + is_file_relative = not isabs(parsed_file_path) # Check if the file path is relative if is_file_relative: parsed_file_path = f"/{parsed_file_path}" return send_file(path_or_file=parsed_file_path) - + # Register access to the provided file path elif request.method == "POST": nwbfile_registry.append(file_path) - return request.base_url # Return the URL of the newly added file + return request.base_url # Return the URL of the newly added file + @flask_app.route("/cpus") def get_cpu_count(): diff --git a/pyflask/utils/_catch_flask_errors.py b/pyflask/utils/_catch_flask_errors.py index a84e50955..79f029a5e 100644 --- a/pyflask/utils/_catch_flask_errors.py +++ b/pyflask/utils/_catch_flask_errors.py @@ -1,7 +1,7 @@ import contextlib +from typing import Dict, List, Union import flask_restx -from typing import List, Union, Dict def server_error_responses(*, codes: List[str]) -> Dict[int, str]: diff --git a/src/renderer/src/stories/pages/preview/PreviewPage.js b/src/renderer/src/stories/pages/preview/PreviewPage.js index d14f3c61a..ebc875382 100644 --- a/src/renderer/src/stories/pages/preview/PreviewPage.js +++ b/src/renderer/src/stories/pages/preview/PreviewPage.js @@ -18,13 +18,11 @@ export class PreviewPage extends Page { updatePath = async (path) => { if (path) { - // Enable access to the explicit file path const result = await fetch(`${baseUrl}/files/${path}`, { method: "POST" }).then((res) => res.text()); // Set Neurosift to access the returned URL if (result) this.neurosift.url = result; - } else this.neurosift.url = undefined; }; diff --git a/src/renderer/src/stories/preview/NWBFilePreview.js b/src/renderer/src/stories/preview/NWBFilePreview.js index a7c3fba8d..9a82e8880 100644 --- a/src/renderer/src/stories/preview/NWBFilePreview.js +++ b/src/renderer/src/stories/preview/NWBFilePreview.js @@ -56,27 +56,26 @@ class NWBPreviewInstance extends LitElement { render() { const isOnline = navigator.onLine; - if (!isOnline) return until( - (async () => { - const htmlRep = await run("html", { nwbfile_path: this.file }, { swal: false }); - return unsafeHTML(htmlRep); - })(), - html`Loading HTML representation...` - ); + if (!isOnline) + return until( + (async () => { + const htmlRep = await run("html", { nwbfile_path: this.file }, { swal: false }); + return unsafeHTML(htmlRep); + })(), + html`Loading HTML representation...` + ); - const neurosift = new Neurosift({ fullscreen: false }) + const neurosift = new Neurosift({ fullscreen: false }); // Enable access to the explicit file path fetch(`${baseUrl}/files/${this.file}`, { method: "POST" }) - .then((res) => res.text()) - .then((result) => { + .then((res) => res.text()) + .then((result) => { + // Set Neurosift to access the returned URL + if (result) neurosift.url = result; + }); - // Set Neurosift to access the returned URL - if (result) neurosift.url = result; - }) - - - return neurosift + return neurosift; } }