diff --git a/.github/workflows/format_and_lint.yaml b/.github/workflows/format_and_lint.yaml new file mode 100644 index 0000000..be0760a --- /dev/null +++ b/.github/workflows/format_and_lint.yaml @@ -0,0 +1,28 @@ +name: Check formatting and linting + +on: + pull_request: + push: { branches: [main] } + +jobs: + ruff-check: + name: Run ruff lint and format checks + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + cache: 'pip' + + - name: Installing dependencies + run: pip install ruff + + - name: Run ruff lint + run: ruff check . + + - name: Run ruff format + run: ruff format . --check + diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..d3db841 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,16 @@ +repos: +- repo: local + hooks: + - id: lint + name: Ruff Lint + description: Linting using ruff + entry: bash -c 'ruff check .' + language: system + stages: ["pre-commit", "pre-push"] + + - id: format + name: Ruff Format + description: Formatting using ruff + entry: bash -c 'ruff format . --check' + language: system + stages: ["pre-commit", "pre-push"] diff --git a/README.md b/README.md index 140eb15..6065aa2 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,8 @@ [![Documentation Status](https://readthedocs.org/projects/fmu-sumo-uploader/badge/?version=latest)](https://fmu-sumo-uploader.readthedocs.io/en/latest/?badge=latest) [![SCM Compliance](https://scm-compliance-api.radix.equinor.com/repos/equinor/fmu-sumo-uploader/badge)](https://scm-compliance-api.radix.equinor.com/repos/equinor/fmu-sumo-uploader/badge) +[![Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/v2.json)](https://github.com/astral-sh/ruff) + ## Documentation and guidelines [fmu-sumo-uploader documentation](https://fmu-sumo-uploader.readthedocs.io/en/latest/) diff --git a/docs/conf.py b/docs/conf.py index 5c73ccc..22d66f7 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -55,4 +55,3 @@ # relative to this directory. They are copied after the builtin static files, # so a file named "default.css" will overwrite the builtin "default.css". html_static_path = ["_static"] - diff --git a/pyproject.toml b/pyproject.toml index f7948df..12f267c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,12 +5,6 @@ build-backend = "setuptools.build_meta" [tool.setuptools_scm] version_file = "src/fmu/sumo/uploader/_version.py" -[tool.isort] -profile = "black" - -[tool.black] -line-length = 79 - [project] name = "fmu-sumo-uploader" requires-python = ">=3.9" @@ -25,7 +19,7 @@ dependencies = [ ] [project.optional-dependencies] -dev = ["black", "flake8", "pytest"] +dev = ["ruff", "pytest", "pre-commit"] test = ["pytest", "pytest-timeout", "fmu-sumo"] docs = [ @@ -50,3 +44,34 @@ sumo_upload = "fmu.sumo.uploader.scripts.sumo_upload:main" [project.entry-points.ert] fmu_sumo_uploader_jobs = "fmu.sumo.uploader.hook_implementations.jobs" sumo_upload = "fmu.sumo.uploader.scripts.sumo_upload" + +[tool.ruff] +exclude = [ + ".env", + ".git", + ".github", + ".venv", + "venv", +] + +line-length = 79 + +[tool.ruff.lint] +ignore = [ + "E501", + "PD901", +] + +extend-select = [ + "C4", # Flake8-comprehensions + "I", # isort + "SIM", # Flake8-simplify + "TC", # Flake8-type-checking + "TID", # Flake8-tidy-imports + "N", # pep8-naming + "PD", # Pandas +] + +[tool.ruff.lint.per-file-ignores] +"__init__.py" = ["F401"] + diff --git a/src/fmu/sumo/uploader/__init__.py b/src/fmu/sumo/uploader/__init__.py index 52da416..bf74dea 100644 --- a/src/fmu/sumo/uploader/__init__.py +++ b/src/fmu/sumo/uploader/__init__.py @@ -7,9 +7,10 @@ except ImportError: __version__ = "0.0.0" +import sumo.wrapper + from fmu.sumo.uploader.caseondisk import CaseOnDisk from fmu.sumo.uploader.caseonjob import CaseOnJob -import sumo.wrapper SumoConnection = sumo.wrapper.SumoClient diff --git a/src/fmu/sumo/uploader/_fileondisk.py b/src/fmu/sumo/uploader/_fileondisk.py index 4e36eeb..7d97b32 100644 --- a/src/fmu/sumo/uploader/_fileondisk.py +++ b/src/fmu/sumo/uploader/_fileondisk.py @@ -1,19 +1,19 @@ """ - The FileOnDisk class objectifies a file as it appears - on the disk. A file in this context refers to a data/metadata - pair (technically two files). +The FileOnDisk class objectifies a file as it appears +on the disk. A file in this context refers to a data/metadata +pair (technically two files). """ -import os -import hashlib import base64 +import hashlib +import os + import yaml -from fmu.sumo.uploader._sumofile import SumoFile, _path_to_yaml_path from fmu.sumo.uploader._logger import get_uploader_logger - +from fmu.sumo.uploader._sumofile import SumoFile, _path_to_yaml_path # pylint: disable=C0103 # allow non-snake case variable names diff --git a/src/fmu/sumo/uploader/_fileonjob.py b/src/fmu/sumo/uploader/_fileonjob.py index 7c15c00..f6c0368 100644 --- a/src/fmu/sumo/uploader/_fileonjob.py +++ b/src/fmu/sumo/uploader/_fileonjob.py @@ -1,22 +1,22 @@ """ - The FileOnDisk class objectifies a file as it appears - on the disk. A file in this context refers to a data/metadata - pair (technically two files). +The FileOnDisk class objectifies a file as it appears +on the disk. A file in this context refers to a data/metadata +pair (technically two files). """ -import hashlib import base64 +import hashlib -from fmu.sumo.uploader._sumofile import SumoFile from fmu.sumo.uploader._logger import get_uploader_logger - +from fmu.sumo.uploader._sumofile import SumoFile # pylint: disable=C0103 # allow non-snake case variable names logger = get_uploader_logger() + class FileOnJob(SumoFile): def __init__(self, byte_string: str, metadata): """ @@ -41,4 +41,3 @@ def __init__(self, byte_string: str, metadata): self.metadata["file"]["checksum_md5"] = digester.hexdigest() self.metadata["file"]["absolute_path"] = "" - diff --git a/src/fmu/sumo/uploader/_logger.py b/src/fmu/sumo/uploader/_logger.py index 228335d..b99f5fb 100644 --- a/src/fmu/sumo/uploader/_logger.py +++ b/src/fmu/sumo/uploader/_logger.py @@ -1,33 +1,38 @@ import logging import sys + # def getLogger(module_name="subscript"): def get_uploader_logger(): # pylint: disable=invalid-name """Provides a unified logger for fmu-sumo-uploader. - Code is copied from + Code is copied from https://github.com/equinor/subscript/blob/main/src/subscript/__init__.py Logging output is split by logging levels (split between WARNING and ERROR) - to stdout and stderr, each log occurs in only one of the streams. + to stdout and stderr, each log occurs in only one of the streams. Returns: A logger object """ - logger = logging.getLogger("fmu.sumo.uploader") - logger.propagate = False # Avoids duplicate logging + logger = logging.getLogger("fmu.sumo.uploader") + logger.propagate = False # Avoids duplicate logging if not len(logger.handlers): - formatter = logging.Formatter("%(asctime)s:%(levelname)s:%(name)s:%(message)s") - + formatter = logging.Formatter( + "%(asctime)s:%(levelname)s:%(name)s:%(message)s" + ) + stdout_handler = logging.StreamHandler(sys.stdout) stdout_handler.addFilter(lambda record: record.levelno < logging.ERROR) stdout_handler.setFormatter(formatter) logger.addHandler(stdout_handler) stderr_handler = logging.StreamHandler(sys.stderr) - stderr_handler.addFilter(lambda record: record.levelno >= logging.ERROR) + stderr_handler.addFilter( + lambda record: record.levelno >= logging.ERROR + ) stderr_handler.setFormatter(formatter) logger.addHandler(stderr_handler) diff --git a/src/fmu/sumo/uploader/_sumocase.py b/src/fmu/sumo/uploader/_sumocase.py index b52a676..3334988 100644 --- a/src/fmu/sumo/uploader/_sumocase.py +++ b/src/fmu/sumo/uploader/_sumocase.py @@ -4,15 +4,13 @@ """ -import warnings -import time import datetime import statistics +import time +import warnings - -from fmu.sumo.uploader._upload_files import upload_files from fmu.sumo.uploader._logger import get_uploader_logger - +from fmu.sumo.uploader._upload_files import upload_files # pylint: disable=C0103 # allow non-snake case variable names @@ -82,7 +80,7 @@ def upload(self, threads=4): ok_uploads = [] failed_uploads = [] rejected_uploads = [] - files_to_upload = [f for f in self.files] + files_to_upload = list(self.files) _t0 = time.perf_counter() @@ -102,23 +100,20 @@ def upload(self, threads=4): rejected_uploads += upload_results.get("rejected_uploads") failed_uploads = upload_results.get("failed_uploads") - if rejected_uploads: - if any( - [ - res.get("metadata_upload_response_status_code") in [404] - for res in rejected_uploads - ] - ): - warnings.warn("Case is not registered on Sumo") - logger.info( - "Case was not found on Sumo. If you are in the FMU context " - "something may have gone wrong with the case registration " - "or you have not specified that the case shall be uploaded." - "A warning will be issued, and the script will stop. " - "If you are NOT in the FMU context, you can specify that " - "this script also registers the case by passing " - "register=True. This should not be done in the FMU context." - ) + if rejected_uploads and any( + res.get("metadata_upload_response_status_code") in [404] + for res in rejected_uploads + ): + warnings.warn("Case is not registered on Sumo") + logger.info( + "Case was not found on Sumo. If you are in the FMU context " + "something may have gone wrong with the case registration " + "or you have not specified that the case shall be uploaded." + "A warning will be issued, and the script will stop. " + "If you are NOT in the FMU context, you can specify that " + "this script also registers the case by passing " + "register=True. This should not be done in the FMU context." + ) _dt = time.perf_counter() - _t0 @@ -218,7 +213,7 @@ def _get_log_msg(sumo_parent_id, status): status.get("blob_upload_response_status_code") ), "response_text": ( - (status.get("blob_upload_response_status_text")) + status.get("blob_upload_response_status_text") ), }, } @@ -268,7 +263,7 @@ def _sanitize_datetimes(data): return data.isoformat() if isinstance(data, dict): - for key in data.keys(): + for key in data: data[key] = _sanitize_datetimes(data[key]) elif isinstance(data, list): diff --git a/src/fmu/sumo/uploader/_sumofile.py b/src/fmu/sumo/uploader/_sumofile.py index c1efcb3..4d997ca 100644 --- a/src/fmu/sumo/uploader/_sumofile.py +++ b/src/fmu/sumo/uploader/_sumofile.py @@ -5,14 +5,15 @@ """ import os +import subprocess import sys import time -import subprocess import warnings + import httpx from azure.storage.blob import BlobClient, ContentSettings -from fmu.sumo.uploader._logger import get_uploader_logger +from fmu.sumo.uploader._logger import get_uploader_logger # pylint: disable=C0103 # allow non-snake case variable names @@ -88,7 +89,7 @@ def _upload_byte_string(self, blob_url): content_settings = ContentSettings( content_type="application/octet-stream" ) - response = blobclient.upload_blob( + blobclient.upload_blob( self.byte_string, blob_type="BlockBlob", length=len(self.byte_string), @@ -135,9 +136,9 @@ def upload_to_sumo(self, sumo_parent_id, sumoclient, sumo_mode): { "status": "rejected", "metadata_upload_response_status_code": 500, - "metadata_upload_response_text": "File upload cannot be attempted; this is a seismic data object but it does not have a value for data.vertical_domain." - } - ) + "metadata_upload_response_text": "File upload cannot be attempted; this is a seismic data object but it does not have a value for data.vertical_domain.", + } + ) return result try: diff --git a/src/fmu/sumo/uploader/_upload_files.py b/src/fmu/sumo/uploader/_upload_files.py index 1763f2a..7300d58 100644 --- a/src/fmu/sumo/uploader/_upload_files.py +++ b/src/fmu/sumo/uploader/_upload_files.py @@ -1,13 +1,15 @@ """ - The function that uploads files. +The function that uploads files. """ +import json from concurrent.futures import ThreadPoolExecutor from copy import deepcopy -import json + import yaml + from fmu.dataio._utils import read_parameters_txt from fmu.dataio.dataio import ExportData from fmu.sumo.uploader._fileonjob import FileOnJob diff --git a/src/fmu/sumo/uploader/caseondisk.py b/src/fmu/sumo/uploader/caseondisk.py index 5cb728d..ccdc336 100644 --- a/src/fmu/sumo/uploader/caseondisk.py +++ b/src/fmu/sumo/uploader/caseondisk.py @@ -1,18 +1,17 @@ """Objectify an FMU case (results) as it appears on the disk.""" -import os -from pathlib import Path import glob import logging +import os import warnings -import httpx +from pathlib import Path +import httpx import yaml -from fmu.sumo.uploader._sumocase import SumoCase from fmu.sumo.uploader._fileondisk import FileOnDisk from fmu.sumo.uploader._logger import get_uploader_logger - +from fmu.sumo.uploader._sumocase import SumoCase logger = get_uploader_logger() diff --git a/src/fmu/sumo/uploader/caseonjob.py b/src/fmu/sumo/uploader/caseonjob.py index feabc1f..ae2569c 100644 --- a/src/fmu/sumo/uploader/caseonjob.py +++ b/src/fmu/sumo/uploader/caseonjob.py @@ -3,10 +3,9 @@ import logging import warnings -from fmu.sumo.uploader._sumocase import SumoCase from fmu.sumo.uploader._fileonjob import FileOnJob from fmu.sumo.uploader._logger import get_uploader_logger - +from fmu.sumo.uploader._sumocase import SumoCase logger = get_uploader_logger() diff --git a/src/fmu/sumo/uploader/forward_models/__init__.py b/src/fmu/sumo/uploader/forward_models/__init__.py index f7af5bd..bd6df53 100644 --- a/src/fmu/sumo/uploader/forward_models/__init__.py +++ b/src/fmu/sumo/uploader/forward_models/__init__.py @@ -1,4 +1,5 @@ import subprocess + from ert import ( ForwardModelStepJSON, ForwardModelStepPlugin, diff --git a/src/fmu/sumo/uploader/hook_implementations/jobs.py b/src/fmu/sumo/uploader/hook_implementations/jobs.py index c7c9fe3..85bbe64 100644 --- a/src/fmu/sumo/uploader/hook_implementations/jobs.py +++ b/src/fmu/sumo/uploader/hook_implementations/jobs.py @@ -5,6 +5,7 @@ from ert.shared.plugins.plugin_manager import hook_implementation from ert.shared.plugins.plugin_response import plugin_response + from fmu.sumo.uploader.forward_models import SumoUpload @@ -25,9 +26,7 @@ def _get_jobs_from_directory(directory): # pylint: disable=no-value-for-parameter @hook_implementation -@plugin_response( - plugin_name="fmu_sumo_uploader" -) # pylint: disable=no-value-for-parameter +@plugin_response(plugin_name="fmu_sumo_uploader") # pylint: disable=no-value-for-parameter def installable_jobs(): return _get_jobs_from_directory("config_jobs") @@ -42,9 +41,7 @@ def _get_module_variable_if_exists(module_name, variable_name, default=""): @hook_implementation -@plugin_response( - plugin_name="fmu_sumo_uploader" -) # pylint: disable=no-value-for-parameter +@plugin_response(plugin_name="fmu_sumo_uploader") # pylint: disable=no-value-for-parameter def job_documentation(job_name): sumo_fmu_jobs = set(installable_jobs().data.keys()) if job_name not in sumo_fmu_jobs: diff --git a/src/fmu/sumo/uploader/scripts/sumo_upload.py b/src/fmu/sumo/uploader/scripts/sumo_upload.py index 355d07b..8057448 100644 --- a/src/fmu/sumo/uploader/scripts/sumo_upload.py +++ b/src/fmu/sumo/uploader/scripts/sumo_upload.py @@ -2,12 +2,15 @@ """Upload data to Sumo from FMU.""" -import warnings -import os import argparse import logging +import os +import warnings from pathlib import Path -from ert.shared.plugins.plugin_manager import hook_implementation # type: ignore + +from ert.shared.plugins.plugin_manager import ( + hook_implementation, # type: ignore +) from ert.shared.plugins.plugin_response import plugin_response # type: ignore try: @@ -16,6 +19,7 @@ from res.job_queue import ErtScript # type: ignore from sumo.wrapper import SumoClient + from fmu.sumo import uploader from fmu.sumo.uploader._logger import get_uploader_logger diff --git a/src/fmu/sumo/uploader/version.py b/src/fmu/sumo/uploader/version.py index d38728e..cb27b26 100644 --- a/src/fmu/sumo/uploader/version.py +++ b/src/fmu/sumo/uploader/version.py @@ -3,6 +3,7 @@ TYPE_CHECKING = False if TYPE_CHECKING: from typing import Tuple, Union + VERSION_TUPLE = Tuple[Union[int, str], ...] else: VERSION_TUPLE = object @@ -12,5 +13,5 @@ __version_tuple__: VERSION_TUPLE version_tuple: VERSION_TUPLE -__version__ = version = '1.0.2' +__version__ = version = "1.0.2" __version_tuple__ = version_tuple = (1, 0, 2) diff --git a/tests/conftest.py b/tests/conftest.py index d855089..39b0389 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -16,4 +16,3 @@ def pytest_generate_tests(metafunc): if "unique_uuid" in metafunc.fixturenames: metafunc.parametrize("unique_uuid", [uuid.uuid4()]) - diff --git a/tests/test_uploader.py b/tests/test_uploader.py index 192166d..820ce07 100644 --- a/tests/test_uploader.py +++ b/tests/test_uploader.py @@ -1,15 +1,17 @@ +import contextlib +import json +import logging import os +import shutil +import subprocess import sys -import pytest import time from pathlib import Path -import logging -import subprocess -import json -import yaml -import shutil +import pytest +import yaml from sumo.wrapper import SumoClient + from fmu.sumo import uploader if not sys.platform.startswith("darwin") and sys.version_info < (3, 12): @@ -28,10 +30,8 @@ def _remove_cached_case_id(): """The sumo uploader caches case uuid on disk, but we should remove this file between tests""" - try: + with contextlib.suppress(FileNotFoundError): os.remove("tests/data/test_case_080/sumo_parent_id.yml") - except FileNotFoundError: - pass def _update_metadata_file_with_unique_uuid(metadata_file, unique_case_uuid): @@ -80,7 +80,7 @@ def test_initialization(token): """Assert that the CaseOnDisk object can be initialized""" sumoclient = SumoClient(env=ENV, token=token) - case = uploader.CaseOnDisk( + uploader.CaseOnDisk( case_metadata_path="tests/data/test_case_080/case.yml", sumoclient=sumoclient, ) @@ -466,7 +466,7 @@ def test_invalid_yml_in_case_metadata(token, unique_uuid): case_file = "tests/data/test_case_080/case_invalid.yml" # Invalid yml file, skip _update_metadata_file_with_unique_uuid(case_file, unique_uuid) with pytest.warns(UserWarning) as warnings_record: - e = uploader.CaseOnDisk( + uploader.CaseOnDisk( case_metadata_path=case_file, sumoclient=sumoclient, ) @@ -622,12 +622,12 @@ def _get_segy_path(segy_command): ) def test_openvds_available(): """Test that OpenVDS is installed and can be successfully called""" - path_to_SEGYImport = _get_segy_path("SEGYImport") - check_SEGYImport_version = subprocess.run( - [path_to_SEGYImport, "--version"], capture_output=True, text=True + path_to_segy_import = _get_segy_path("SEGYImport") + check_segy_import_version = subprocess.run( + [path_to_segy_import, "--version"], capture_output=True, text=True ) - assert check_SEGYImport_version.returncode == 0 - assert "SEGYImport" in check_SEGYImport_version.stdout + assert check_segy_import_version.returncode == 0 + assert "SEGYImport" in check_segy_import_version.stdout @pytest.mark.skipif( @@ -688,7 +688,7 @@ def test_seismic_openvds_file(token, unique_uuid): url_conn = "Suffix=?" + json.loads(token_results.decode("utf-8")).get( "auth" ) - except: + except: # noqa: E722 token_results = token_results.decode("utf-8") url = "azureSAS" + token_results.split("?")[0][5:] + "/" url_conn = "Suffix=?" + token_results.split("?")[1] @@ -705,9 +705,9 @@ def test_seismic_openvds_file(token, unique_uuid): exported_filepath = "exported.segy" if os.path.exists(exported_filepath): os.remove(exported_filepath) - path_to_SEGYExport = _get_segy_path("SEGYExport") + path_to_segy_export = _get_segy_path("SEGYExport") cmdstr = [ - path_to_SEGYExport, + path_to_segy_export, "--url", url, "--connection", diff --git a/tests/test_uploads_from_komodo.py b/tests/test_uploads_from_komodo.py index 6310005..21381a1 100644 --- a/tests/test_uploads_from_komodo.py +++ b/tests/test_uploads_from_komodo.py @@ -1,23 +1,24 @@ -""" Verifies SUMO uploads that has been run by Github Actions in the - komodo-releases repo. See - https://github.com/equinor/komodo-releases/blob/main/.github/workflows/run_drogon.yml - - Verify that the expected number of objects of each expected type have been - uploaded. Note that komodo-releases repo usually runs multiple times every - day, and that many failed runs is to be expected, i.e. do not expect - every upload to be perfect. - Tests of aggregations are not in scope here, see the Sumo aggregation repo - for those tests. - """ +"""Verifies SUMO uploads that has been run by Github Actions in the +komodo-releases repo. See +https://github.com/equinor/komodo-releases/blob/main/.github/workflows/run_drogon.yml + +Verify that the expected number of objects of each expected type have been +uploaded. Note that komodo-releases repo usually runs multiple times every +day, and that many failed runs is to be expected, i.e. do not expect +every upload to be perfect. +Tests of aggregations are not in scope here, see the Sumo aggregation repo +for those tests. +""" +import logging import os import sys from datetime import datetime, timedelta, timezone -import pytest from pathlib import Path -import logging -from random import seed -from random import randint +from random import randint, seed + +import pytest + from fmu.sumo.explorer import Explorer if not sys.platform.startswith("darwin"): @@ -106,8 +107,8 @@ def test_case_consistency(explorer: Explorer): res = explorer._sumo.get(f"/admin/consistency-check?case={case.uuid}") metadata_wo_blobs = len(res.json().get("metadata_without_blobs")) blobs_wo_metadata = len(res.json().get("blobs_without_metadata")) - if (metadata_wo_blobs > 0 or blobs_wo_metadata > 0): - print ("NOT consistent case:", case.uuid, res.json()) + if metadata_wo_blobs > 0 or blobs_wo_metadata > 0: + print("NOT consistent case:", case.uuid, res.json()) non_consistent_cases += 1 print(f"{non_consistent_cases} NON-consistent cases out of {len(cases)}")