From 5854197d4dddcfeed3f10f7d4e39708d44f15bb9 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 17 Jan 2025 09:27:36 +0000 Subject: [PATCH 01/12] Modernise layout --- .gitignore | 4 +++ pyproject.toml | 76 ++++++++++++++++++++++++++++++++++++++++++++++++ requirements.txt | 9 ------ 3 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 pyproject.toml delete mode 100644 requirements.txt diff --git a/.gitignore b/.gitignore index 808214e..080f414 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,7 @@ relPaths.sh *.kdb *.kdbx /exp_db_populator_venv +/.venv +/.coverage +*.egg-info +/logs diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..e1383b1 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,76 @@ +[build-system] +requires = ["setuptools>=64", "setuptools_scm>=8"] +build-backend = "setuptools.build_meta" + + +[project] +name = "ExperimentDetailsPopulator" +dynamic = ["version"] +description = "Experiment Details Populator" +readme = "README.md" +requires-python = ">=3.10" +license = {file = "LICENSE"} + +authors = [ + {name = "ISIS Experiment Controls", email = "ISISExperimentControls@stfc.ac.uk" } +] +maintainers = [ + {name = "ISIS Experiment Controls", email = "ISISExperimentControls@stfc.ac.uk" } +] + +classifiers = [ + "Development Status :: 5 - Production/Stable", + "Intended Audience :: Developers", + "License :: OSI Approved :: BSD License", + "Programming Language :: Python :: 3.10", +] + +dependencies = [ + "suds", + "pykeepass", + "peewee", + "pymysql", + "pyepics", + "cryptography", + "six", +] + +[project.optional-dependencies] + +dev = [ + "pytest", + "pytest-cov", + "ruff>=0.9", + "mock", + "xmlrunner", +] + +[project.urls] +"Homepage" = "https://github.com/isiscomputinggroup/ExperimentDatabasePopulator" +"Bug Reports" = "https://github.com/isiscomputinggroup/ExperimentDatabasePopulator/issues" +"Source" = "https://github.com/isiscomputinggroup/ExperimentDatabasePopulator" + +[tool.pytest.ini_options] +testpaths = "tests" +addopts = "--cov --cov-report=html -vv" + +[tool.coverage.run] +branch = true +source = ["exp_db_populator"] + +[tool.coverage.report] +exclude_lines = [ + "pragma: no cover", + "if TYPE_CHECKING:", + "if typing.TYPE_CHECKING:", + "@abstractmethod", +] + +[tool.coverage.html] +directory = "coverage_html_report" + +[tool.setuptools_scm] + +[tool.setuptools.packages.find] +include = ["exp_db_populator"] +namespaces = false diff --git a/requirements.txt b/requirements.txt deleted file mode 100644 index 3c0dd22..0000000 --- a/requirements.txt +++ /dev/null @@ -1,9 +0,0 @@ -suds-jurko -pykeepass -peewee -pymysql -mock -xmlrunner -pyepics -six -cryptography \ No newline at end of file From 64771e25d559fbc2b9f30704c5d00a4608b9250c Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 17 Jan 2025 09:30:06 +0000 Subject: [PATCH 02/12] Add ruff config --- ruff.toml | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 ruff.toml diff --git a/ruff.toml b/ruff.toml new file mode 100644 index 0000000..21200fa --- /dev/null +++ b/ruff.toml @@ -0,0 +1,37 @@ +# Exclude a variety of commonly ignored directories. +exclude = [ + ".pyi", + "exp_db_populator/passwords" +] + +# Set the maximum line length to 100. +line-length = 100 +indent-width = 4 + +[lint] +extend-select = [ + "N", # pep8-naming + # "D", # pydocstyle (can use this later but for now causes too many errors) + "I", # isort (for imports) + "E501", # Line too long ({width} > {limit}) + "E", + "F", + "ANN", +] +ignore = [ + "D406", # Section name should end with a newline ("{name}") + "D407", # Missing dashed underline after section ("{name}") +] +[lint.per-file-ignores] +"tests/*" = [ + "N802", + "D100", + "D101", + "D102", + "E501", + "ANN", +] + +[lint.pydocstyle] +# Use Google-style docstrings. +convention = "google" From 9d75e8abf35e2f8f9d9a1ddb56791dc3934c66a1 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 17 Jan 2025 09:53:57 +0000 Subject: [PATCH 03/12] Ruff checks --- exp_db_populator/data_types.py | 6 ++++-- exp_db_populator/database_model.py | 11 +++++++++- exp_db_populator/gatherer.py | 3 ++- main.py => exp_db_populator/main.py | 17 ++++++++------- exp_db_populator/populator.py | 19 ++++++++++------ exp_db_populator/webservices_reader.py | 5 +++-- pyproject.toml | 3 +++ ruff.toml | 6 +++++- tests/test_data_types.py | 6 +++--- tests/test_gatherer.py | 3 +-- tests/test_main.py | 2 +- tests/test_populator.py | 30 ++++++++++++++++---------- tests/test_webservices_reader.py | 25 ++++++++++++++++++--- 13 files changed, 94 insertions(+), 42 deletions(-) rename main.py => exp_db_populator/main.py (96%) diff --git a/exp_db_populator/data_types.py b/exp_db_populator/data_types.py index 9d536e6..3e2879c 100644 --- a/exp_db_populator/data_types.py +++ b/exp_db_populator/data_types.py @@ -19,7 +19,8 @@ def __str__(self): @property def user_id(self): """ - Gets the user id for the user. Will create an entry for the user in the database if one doesn't exist. + Gets the user id for the user. Will create an entry for the user in + the database if one doesn't exist. Returns: the user's id. """ @@ -43,7 +44,8 @@ def __init__(self, user, role, rb_number, start_date): @property def role_id(self): """ - Gets the role id for the user based on the roles in the database. Will raise an exception if role is not found. + Gets the role id for the user based on the roles in the database. + Will raise an exception if role is not found. Returns: the role id. diff --git a/exp_db_populator/database_model.py b/exp_db_populator/database_model.py index 1a5944c..5f1a784 100644 --- a/exp_db_populator/database_model.py +++ b/exp_db_populator/database_model.py @@ -1,4 +1,13 @@ -from peewee import * +from peewee import ( + AutoField, + CharField, + CompositeKey, + DateTimeField, + ForeignKeyField, + IntegerField, + Model, + Proxy, +) # Model built using peewiz diff --git a/exp_db_populator/gatherer.py b/exp_db_populator/gatherer.py index 885e7d2..a0329ab 100644 --- a/exp_db_populator/gatherer.py +++ b/exp_db_populator/gatherer.py @@ -59,7 +59,8 @@ def run(self): instrument_list = filter_instrument_data(all_data, name) if not instrument_list: logging.error( - f"Unable to update {name}, no data found. Expired data will still be cleared." + f"Unable to update {name}, no data found. " + f"Expired data will still be cleared." ) data_to_populate = None else: diff --git a/main.py b/exp_db_populator/main.py similarity index 96% rename from main.py rename to exp_db_populator/main.py index dd04b76..a1f762b 100644 --- a/main.py +++ b/exp_db_populator/main.py @@ -24,28 +24,28 @@ import epics from six.moves import input - -from exp_db_populator.gatherer import Gatherer -from exp_db_populator.populator import update -from exp_db_populator.webservices_reader import reformat_data from tests.webservices_test_data import ( TEST_USER_1, create_web_data_with_experimenters_and_other_date, ) +from exp_db_populator.gatherer import Gatherer +from exp_db_populator.populator import update +from exp_db_populator.webservices_reader import reformat_data + # PV that contains the instrument list INST_LIST_PV = "CS:INSTLIST" -def convert_inst_list(value_from_PV): +def convert_inst_list(value_from_pv): """ Converts the instrument list coming from the PV into a dictionary. Args: - value_from_PV: The raw value from the PV. + value_from_pv: The raw value from the PV. Returns: dict: The instrument information. """ - json_string = zlib.decompress(bytes.fromhex(value_from_PV)).decode("utf-8") + json_string = zlib.decompress(bytes.fromhex(value_from_pv)).decode("utf-8") return json.loads(json_string) @@ -71,7 +71,8 @@ def inst_list_callback(self, char_value, **kw): Called when the instrument list PV changes value. Args: char_value: The string representation of the PV data. - **kw: The module will also send other info about the PV, we capture this and don't use it. + **kw: The module will also send other info about the PV, we capture this and don't + use it. """ new_inst_list = convert_inst_list(char_value) if new_inst_list != self.prev_inst_list: diff --git a/exp_db_populator/populator.py b/exp_db_populator/populator.py index 237f032..89166d1 100644 --- a/exp_db_populator/populator.py +++ b/exp_db_populator/populator.py @@ -14,8 +14,11 @@ "unless username/password are specified manually" ) -AGE_OF_EXPIRATION = 100 # How old (in days) the startdate of an experiment must be before it is removed from the database -POLLING_TIME = 3600 # Time in seconds between polling the website +# How old (in days) the startdate of an experiment must be before it is removed from the database +AGE_OF_EXPIRATION = 100 + +# Time in seconds between polling the website +POLLING_TIME = 3600 def remove_users_not_referenced(): @@ -56,7 +59,8 @@ def populate(experiments, experiment_teams): Args: experiments (list[dict]): A list of dictionaries containing information on experiments. - experiment_teams (list[exp_db_populator.data_types.ExperimentTeamData]): A list containing the users for all new experiments. + experiment_teams (list[exp_db_populator.data_types.ExperimentTeamData]): A list containing + the users for all new experiments. """ if not experiments or not experiment_teams: raise KeyError("Experiment without team or vice versa") @@ -93,10 +97,11 @@ def update( instrument_name: The name of the instrument to update. instrument_host: The host name of the instrument to update. db_lock: A lock for writing to the database. - instrument_data: The data to send to the instrument, if None the data will just be cleared instead. - run_continuous: Whether or not the program is running in continuous mode. - credentials: The credentials to write to the database with, in the form (user, password). If None then the - credentials are received from the stored git repo + instrument_data: The data to send to the instrument, if None the data will just be + cleared instead. + run_continuous: Whether the program is running in continuous mode. + credentials: The credentials to write to the database with, in the form (user, password). + If None then the credentials are received from the stored git repo """ database = create_database(instrument_host, credentials) logging.info( diff --git a/exp_db_populator/webservices_reader.py b/exp_db_populator/webservices_reader.py index 6873292..dfaaa49 100644 --- a/exp_db_populator/webservices_reader.py +++ b/exp_db_populator/webservices_reader.py @@ -104,8 +104,9 @@ def reformat_data(instrument_data_list): instrument_data_list (list): List of an instrument's data from the website. Returns: - tuple (list, list): A list of the experiments and their associated data and a list of the experiment teams, - and a dictionary of rb_numbers and their associated instrument.. + tuple (list, list): A list of the experiments and their associated data and a + list of the experiment teams, and a dictionary of rb_numbers and their associated + instrument. """ try: experiments = [] diff --git a/pyproject.toml b/pyproject.toml index e1383b1..3b5ece7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -74,3 +74,6 @@ directory = "coverage_html_report" [tool.setuptools.packages.find] include = ["exp_db_populator"] namespaces = false + +[project.scripts] +main = "exp_db_populator:main" diff --git a/ruff.toml b/ruff.toml index 21200fa..8ded9ff 100644 --- a/ruff.toml +++ b/ruff.toml @@ -4,6 +4,8 @@ exclude = [ "exp_db_populator/passwords" ] +src = ["exp_db_populator"] + # Set the maximum line length to 100. line-length = 100 indent-width = 4 @@ -16,7 +18,6 @@ extend-select = [ "E501", # Line too long ({width} > {limit}) "E", "F", - "ANN", ] ignore = [ "D406", # Section name should end with a newline ("{name}") @@ -31,6 +32,9 @@ ignore = [ "E501", "ANN", ] +"exp_db_populator/main.py" = [ + "E402" +] [lint.pydocstyle] # Use Google-style docstrings. diff --git a/tests/test_data_types.py b/tests/test_data_types.py index 9093599..4bdbb55 100644 --- a/tests/test_data_types.py +++ b/tests/test_data_types.py @@ -1,10 +1,10 @@ import unittest -from peewee import SqliteDatabase - import exp_db_populator.database_model as model from exp_db_populator.data_types import ExperimentTeamData, UserData -from tests.webservices_test_data import * +from peewee import SqliteDatabase + +from .webservices_test_data import TEST_DATE, TEST_PI_NAME, TEST_PI_ORG, TEST_PI_ROLE, TEST_RBNUMBER class UserDataTests(unittest.TestCase): diff --git a/tests/test_gatherer.py b/tests/test_gatherer.py index a1d2cb2..d1f1cf9 100644 --- a/tests/test_gatherer.py +++ b/tests/test_gatherer.py @@ -1,9 +1,8 @@ import threading import unittest -from mock import patch - from exp_db_populator.gatherer import Gatherer, filter_instrument_data +from mock import patch class GathererTests(unittest.TestCase): diff --git a/tests/test_main.py b/tests/test_main.py index 2c5bf7a..070594a 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1,8 +1,8 @@ import unittest +from exp_db_populator.gatherer import Gatherer from mock import Mock, patch -from exp_db_populator.gatherer import Gatherer from main import InstrumentPopulatorRunner diff --git a/tests/test_populator.py b/tests/test_populator.py index 406daa8..ea3cb83 100644 --- a/tests/test_populator.py +++ b/tests/test_populator.py @@ -1,10 +1,8 @@ import threading import unittest +from datetime import datetime from time import sleep -from mock import Mock, patch -from peewee import SqliteDatabase - import exp_db_populator.database_model as model from exp_db_populator.data_types import ExperimentTeamData, UserData from exp_db_populator.populator import ( @@ -14,7 +12,17 @@ remove_users_not_referenced, update, ) -from tests.webservices_test_data import * +from mock import Mock, patch +from peewee import SqliteDatabase + +from .webservices_test_data import ( + TEST_DATE, + TEST_INSTRUMENT, + TEST_PI_ROLE, + TEST_RBNUMBER, + TEST_TIMEALLOCATED, + TEST_USER_PI, +) class PopulatorTests(unittest.TestCase): @@ -58,15 +66,15 @@ def test_GIVEN_user_and_unrelated_experiment_teams_WHEN_unreferenced_removed_THE self, ): model.User.create(name="Delete me", organisation="STFC") - KEEP_NAME = "Keep Me" - self.create_full_record(user_name=KEEP_NAME) + keep_name = "Keep Me" + self.create_full_record(user_name=keep_name) self.assertEqual(2, model.User.select().count()) remove_users_not_referenced() users = model.User.select() self.assertEqual(1, users.count()) - self.assertEqual(KEEP_NAME, users[0].name) + self.assertEqual(keep_name, users[0].name) def test_GIVEN_user_and_related_experiment_teams_WHEN_unreferenced_removed_THEN_user_remains( self, @@ -90,15 +98,15 @@ def test_GIVEN_experiment_and_unrelated_experiment_teams_WHEN_unreferenced_remov self, ): model.Experiment.create(experimentid=TEST_RBNUMBER, duration=2, startdate=TEST_DATE) - KEEP_RB = "20000" - self.create_full_record(rb_number=KEEP_RB) + keep_rb = "20000" + self.create_full_record(rb_number=keep_rb) self.assertEqual(2, model.Experiment.select().count()) remove_experiments_not_referenced() exps = model.Experiment.select() self.assertEqual(1, exps.count()) - self.assertEqual(KEEP_RB, exps[0].experimentid) + self.assertEqual(keep_rb, exps[0].experimentid) def test_GIVEN_experiment_and_related_experiment_teams_WHEN_unreferenced_removed_THEN_experiment_remains( self, @@ -133,7 +141,7 @@ def create_experiment_teams_dictionary(self): exp_team_data.user.user_id = 1 return [exp_team_data] - def test_WHEN_populate_called_with_experiments_and_no_teams_THEN_exception_raised(self): + def test_WHEN_populate_called_with_experiments_and_no_teams_2THEN_exception_raised(self): experiments = self.create_experiments_dictionary() self.assertRaises(KeyError, populate, experiments, []) diff --git a/tests/test_webservices_reader.py b/tests/test_webservices_reader.py index 56e439c..94e0c6c 100644 --- a/tests/test_webservices_reader.py +++ b/tests/test_webservices_reader.py @@ -1,8 +1,6 @@ import unittest from datetime import datetime, timedelta -from mock import MagicMock - from exp_db_populator.data_types import ExperimentTeamData, UserData from exp_db_populator.database_model import Experiment from exp_db_populator.webservices_reader import ( @@ -13,7 +11,28 @@ get_start_and_end, reformat_data, ) -from tests.webservices_test_data import * +from mock import MagicMock + +from tests.webservices_test_data import ( + TEST_CONTACT_NAME, + TEST_CONTACTS, + TEST_DATA, + TEST_DATE, + TEST_PI_NAME, + TEST_PI_ORG, + TEST_PI_ROLE, + TEST_RBNUMBER, + TEST_TIMEALLOCATED, + TEST_USER_1, + TEST_USER_1_NAME, + TEST_USER_1_ORG, + TEST_USER_1_ROLE, + TEST_USER_PI, + create_data, + create_web_data_with_experimenters, + create_web_data_with_experimenters_and_other_date, + get_test_experiment_team, +) class WebServicesReaderTests(unittest.TestCase): From 15d693004a3a8f1d3729403e0547a7ea770f7306 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 17 Jan 2025 10:11:49 +0000 Subject: [PATCH 04/12] Refactor cli as script --- .gitignore | 2 +- exp_db_populator/{main.py => cli.py} | 18 +++++++++++------- .../webservices_test_data.py | 0 pyproject.toml | 6 +++--- ruff.toml | 2 +- tests/test_data_types.py | 9 +++++++-- tests/test_main.py | 7 +++---- tests/test_populator.py | 7 +++---- tests/test_webservices_reader.py | 5 ++--- 9 files changed, 31 insertions(+), 25 deletions(-) rename exp_db_populator/{main.py => cli.py} (97%) rename {tests => exp_db_populator}/webservices_test_data.py (100%) diff --git a/.gitignore b/.gitignore index 080f414..26d3ed8 100644 --- a/.gitignore +++ b/.gitignore @@ -20,4 +20,4 @@ relPaths.sh /.venv /.coverage *.egg-info -/logs +logs diff --git a/exp_db_populator/main.py b/exp_db_populator/cli.py similarity index 97% rename from exp_db_populator/main.py rename to exp_db_populator/cli.py index a1f762b..734cc36 100644 --- a/exp_db_populator/main.py +++ b/exp_db_populator/cli.py @@ -3,6 +3,11 @@ from datetime import datetime from logging.handlers import TimedRotatingFileHandler +from exp_db_populator.webservices_test_data import ( + TEST_USER_1, + create_web_data_with_experimenters_and_other_date, +) + # Loging must be handled here as some imports might log errors log_folder = os.path.join(os.path.dirname(os.path.realpath(__file__)), "logs") if not os.path.exists(log_folder): @@ -23,11 +28,6 @@ import zlib import epics -from six.moves import input -from tests.webservices_test_data import ( - TEST_USER_1, - create_web_data_with_experimenters_and_other_date, -) from exp_db_populator.gatherer import Gatherer from exp_db_populator.populator import update @@ -110,7 +110,7 @@ def wait_for_gatherer_to_finish(self): self.gatherer.join() -if __name__ == "__main__": +def main_cli(): parser = argparse.ArgumentParser() parser.add_argument( "--cont", @@ -142,7 +142,7 @@ def wait_for_gatherer_to_finish(self): main.prev_inst_list = debug_inst_list main.inst_list_changes(debug_inst_list) elif args.test_data: - data = [create_web_data_with_experimenters_and_other_date([TEST_USER_1], datetime.now())] + data = [create_web_data_with_experimenters_and_other_date(TEST_USER_1, datetime.now())] update( "localhost", "localhost", @@ -170,3 +170,7 @@ def wait_for_gatherer_to_finish(self): logging.warning("Command not recognised: {}".format(menu_input)) else: main.wait_for_gatherer_to_finish() + + +if __name__ == "__main__": + main_cli() diff --git a/tests/webservices_test_data.py b/exp_db_populator/webservices_test_data.py similarity index 100% rename from tests/webservices_test_data.py rename to exp_db_populator/webservices_test_data.py diff --git a/pyproject.toml b/pyproject.toml index 3b5ece7..2a85193 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -50,6 +50,9 @@ dev = [ "Bug Reports" = "https://github.com/isiscomputinggroup/ExperimentDatabasePopulator/issues" "Source" = "https://github.com/isiscomputinggroup/ExperimentDatabasePopulator" +[project.scripts] +exp_db_populator = "exp_db_populator.cli:main_cli" + [tool.pytest.ini_options] testpaths = "tests" addopts = "--cov --cov-report=html -vv" @@ -74,6 +77,3 @@ directory = "coverage_html_report" [tool.setuptools.packages.find] include = ["exp_db_populator"] namespaces = false - -[project.scripts] -main = "exp_db_populator:main" diff --git a/ruff.toml b/ruff.toml index 8ded9ff..fc6aedb 100644 --- a/ruff.toml +++ b/ruff.toml @@ -32,7 +32,7 @@ ignore = [ "E501", "ANN", ] -"exp_db_populator/main.py" = [ +"exp_db_populator/cli.py" = [ "E402" ] diff --git a/tests/test_data_types.py b/tests/test_data_types.py index 4bdbb55..c30a9f4 100644 --- a/tests/test_data_types.py +++ b/tests/test_data_types.py @@ -2,10 +2,15 @@ import exp_db_populator.database_model as model from exp_db_populator.data_types import ExperimentTeamData, UserData +from exp_db_populator.webservices_test_data import ( + TEST_DATE, + TEST_PI_NAME, + TEST_PI_ORG, + TEST_PI_ROLE, + TEST_RBNUMBER, +) from peewee import SqliteDatabase -from .webservices_test_data import TEST_DATE, TEST_PI_NAME, TEST_PI_ORG, TEST_PI_ROLE, TEST_RBNUMBER - class UserDataTests(unittest.TestCase): def setUp(self): diff --git a/tests/test_main.py b/tests/test_main.py index 070594a..13284d2 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -1,16 +1,15 @@ import unittest +from exp_db_populator.cli import InstrumentPopulatorRunner from exp_db_populator.gatherer import Gatherer from mock import Mock, patch -from main import InstrumentPopulatorRunner - class MainTest(unittest.TestCase): def setUp(self): self.inst_pop_runner = InstrumentPopulatorRunner() - @patch("main.Gatherer") + @patch("exp_db_populator.cli.Gatherer") def test_GIVEN_no_gatherer_running_WHEN_instrument_list_has_new_instrument_THEN_gatherer_starts( self, gatherer ): @@ -23,7 +22,7 @@ def test_GIVEN_no_gatherer_running_WHEN_instrument_list_has_new_instrument_THEN_ self.assertEqual(new_gather, self.inst_pop_runner.gatherer) - @patch("main.InstrumentPopulatorRunner.remove_gatherer") + @patch("exp_db_populator.cli.InstrumentPopulatorRunner.remove_gatherer") def test_WHEN_instrument_list_updated_THEN_gatherer_stopped_and_cleared(self, stop): new_name, new_host = "TEST", "NDXTEST" self.inst_pop_runner.inst_list_changes( diff --git a/tests/test_populator.py b/tests/test_populator.py index ea3cb83..d0b2d0e 100644 --- a/tests/test_populator.py +++ b/tests/test_populator.py @@ -12,10 +12,7 @@ remove_users_not_referenced, update, ) -from mock import Mock, patch -from peewee import SqliteDatabase - -from .webservices_test_data import ( +from exp_db_populator.webservices_test_data import ( TEST_DATE, TEST_INSTRUMENT, TEST_PI_ROLE, @@ -23,6 +20,8 @@ TEST_TIMEALLOCATED, TEST_USER_PI, ) +from mock import Mock, patch +from peewee import SqliteDatabase class PopulatorTests(unittest.TestCase): diff --git a/tests/test_webservices_reader.py b/tests/test_webservices_reader.py index 94e0c6c..36d3cce 100644 --- a/tests/test_webservices_reader.py +++ b/tests/test_webservices_reader.py @@ -11,9 +11,7 @@ get_start_and_end, reformat_data, ) -from mock import MagicMock - -from tests.webservices_test_data import ( +from exp_db_populator.webservices_test_data import ( TEST_CONTACT_NAME, TEST_CONTACTS, TEST_DATA, @@ -33,6 +31,7 @@ create_web_data_with_experimenters_and_other_date, get_test_experiment_team, ) +from mock import MagicMock class WebServicesReaderTests(unittest.TestCase): From 0c65cfb98e28b68e56a259282b09c8da43125256 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 17 Jan 2025 16:04:15 +0000 Subject: [PATCH 05/12] Migrate auth to REST --- exp_db_populator/gatherer.py | 3 +++ exp_db_populator/webservices_reader.py | 22 ++++++++++++++-------- pyproject.toml | 1 + 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/exp_db_populator/gatherer.py b/exp_db_populator/gatherer.py index a0329ab..eb45719 100644 --- a/exp_db_populator/gatherer.py +++ b/exp_db_populator/gatherer.py @@ -53,6 +53,9 @@ def run(self): """ while self.running: all_data = gather_data() + + print(all_data) + for inst in self.inst_list: if inst["isScheduled"]: name, host = correct_name(inst["name"]), inst["hostName"] diff --git a/exp_db_populator/webservices_reader.py b/exp_db_populator/webservices_reader.py index dfaaa49..8210b7d 100644 --- a/exp_db_populator/webservices_reader.py +++ b/exp_db_populator/webservices_reader.py @@ -1,9 +1,9 @@ # -*- coding: utf-8 -*- import logging import math -import ssl from datetime import datetime, timedelta +import requests from suds.client import Client from exp_db_populator.data_types import CREDS_GROUP, ExperimentTeamData, UserData @@ -19,13 +19,11 @@ RELEVANT_DATE_RANGE = 100 # How many days of data to gather (either side of now) DATE_TIME_FORMAT = "%Y-%m-%dT%H:%M:%S" -BUS_APPS_SITE = "https://api.facilities.rl.ac.uk/ws/" -BUS_APPS_AUTH = BUS_APPS_SITE + "UserOfficeWebService?wsdl" -BUS_APPS_API = BUS_APPS_SITE + "ScheduleWebService?wsdl" +BUS_APPS_SITE = "https://api.facilities.rl.ac.uk/" +BUS_APPS_AUTH = BUS_APPS_SITE + "users-service/v1/sessions" +BUS_APPS_API = BUS_APPS_SITE + "ws/ScheduleWebService?wsdl" -# This is a workaround because the web service does not have a valid certificate -if hasattr(ssl, "_create_unverified_context"): - ssl._create_default_https_context = ssl._create_unverified_context +SUCCESSFUL_LOGIN_STATUS_CODE = 201 def get_start_and_end(date, time_range_days): @@ -60,7 +58,15 @@ def connect(): try: username, password = get_credentials(CREDS_GROUP, "WebRead") - session_id = Client(BUS_APPS_AUTH).service.login(username, password) + response = requests.post(BUS_APPS_AUTH, json={"username": username, "password": password}) + + if response.status_code != SUCCESSFUL_LOGIN_STATUS_CODE: + raise IOError( + f"Failed to authenticate to busapps web service, " + f"code={response.status_code}, resp={response.text}" + ) + + session_id = response.json()["sessionId"] client = Client(BUS_APPS_API) return client, session_id diff --git a/pyproject.toml b/pyproject.toml index 2a85193..6806c00 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,6 +26,7 @@ classifiers = [ ] dependencies = [ + "requests", "suds", "pykeepass", "peewee", From 167d625426c798ad8ed98b3fe36c76788940df4b Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 17 Jan 2025 16:09:50 +0000 Subject: [PATCH 06/12] Run CI in GHA --- .github/workflows/Lint-and-test.yml | 42 +++++++++++++++++++++++++++++ .github/workflows/linters.yml | 7 ----- 2 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 .github/workflows/Lint-and-test.yml delete mode 100644 .github/workflows/linters.yml diff --git a/.github/workflows/Lint-and-test.yml b/.github/workflows/Lint-and-test.yml new file mode 100644 index 0000000..95faa8b --- /dev/null +++ b/.github/workflows/Lint-and-test.yml @@ -0,0 +1,42 @@ +name: Lint-and-test +on: [pull_request, workflow_call] +jobs: + call-linter-workflow: + uses: ISISComputingGroup/reusable-workflows/.github/workflows/linters.yml@main + with: + compare-branch: origin/main + python-ver: '3.11' + tests: + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [ "ubuntu-latest" ] + # Wide matrix of versions as this may run on a RHEL node with old python versions, + # but we also want it to work on dev machines + version: ['3.8', '3.9', '3.10', '3.11', '3.12', '3.13'] + include: + - os: "windows-latest" + version: '3.11' + fail-fast: false + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.version }} + - name: install requirements + run: pip install -e .[dev] + - name: run pytest (linux) + run: python -m pytest + results: + if: ${{ always() }} + runs-on: ubuntu-latest + name: Final Results + needs: [call-linter-workflow, tests] + steps: + - run: exit 1 + # see https://stackoverflow.com/a/67532120/4907315 + if: >- + ${{ + contains(needs.*.result, 'failure') + || contains(needs.*.result, 'cancelled') + }} diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml deleted file mode 100644 index 82ad245..0000000 --- a/.github/workflows/linters.yml +++ /dev/null @@ -1,7 +0,0 @@ -name: Linter -on: [pull_request] -jobs: - call-workflow: - uses: ISISComputingGroup/reusable-workflows/.github/workflows/linters.yml@main - with: - compare-branch: origin/master From 239b7161be1d6cb389aa08d83f06f720d4700048 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 17 Jan 2025 16:15:37 +0000 Subject: [PATCH 07/12] Correct CI --- .github/workflows/Lint-and-test.yml | 2 +- .gitignore | 1 + Jenkinsfile | 74 ----------------------------- pyproject.toml | 13 ++--- rb_number_populator.sh | 2 +- run_tests.py | 37 --------------- 6 files changed, 8 insertions(+), 121 deletions(-) delete mode 100644 Jenkinsfile delete mode 100644 run_tests.py diff --git a/.github/workflows/Lint-and-test.yml b/.github/workflows/Lint-and-test.yml index 95faa8b..f128a43 100644 --- a/.github/workflows/Lint-and-test.yml +++ b/.github/workflows/Lint-and-test.yml @@ -4,7 +4,7 @@ jobs: call-linter-workflow: uses: ISISComputingGroup/reusable-workflows/.github/workflows/linters.yml@main with: - compare-branch: origin/main + compare-branch: origin/master python-ver: '3.11' tests: runs-on: ${{ matrix.os }} diff --git a/.gitignore b/.gitignore index 26d3ed8..ebc0efd 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,4 @@ relPaths.sh /.coverage *.egg-info logs +/build diff --git a/Jenkinsfile b/Jenkinsfile deleted file mode 100644 index e0be109..0000000 --- a/Jenkinsfile +++ /dev/null @@ -1,74 +0,0 @@ -#!groovy - -pipeline { - - // agent defines where the pipeline will run. - agent { - label { - label "Python_3_tests" - // Use custom workspace to avoid issue with long filepaths on Win32 - customWorkspace "C:/Exp_DB_Populator/${env.BRANCH_NAME}" - } - } - - triggers { - pollSCM('H/2 * * * *') - } - - stages { - stage("Checkout") { - steps { - echo "Branch: ${env.BRANCH_NAME}" - checkout scm - } - } - - stage("Build") { - steps { - echo "Build Number: ${env.BUILD_NUMBER}" - script { - env.GIT_COMMIT = bat(returnStdout: true, script: '@git rev-parse HEAD').trim() - env.GIT_BRANCH = bat(returnStdout: true, script: '@git rev-parse --abbrev-ref HEAD').trim() - echo "git commit: ${env.GIT_COMMIT}" - echo "git branch: ${env.BRANCH_NAME} ${env.GIT_BRANCH}" - if (env.BRANCH_NAME != null && env.BRANCH_NAME.startsWith("Release")) { - env.IS_RELEASE = "YES" - env.RELEASE_VERSION = "${env.BRANCH_NAME}".replace('Release_', '') - echo "release version: ${env.RELEASE_VERSION}" - } - else { - env.IS_RELEASE = "NO" - env.RELEASE_VERSION = "" - } - } - - bat """ - C:/Instrument/Apps/Python3/python.exe -m pip install virtualenv - C:/Instrument/Apps/Python3/Scripts/pip.exe install virtualenv - C:/Instrument/Apps/Python3/Scripts/virtualenv.exe my_python - call my_python\\Scripts\\activate.bat - call my_python\\Scripts\\pip.exe install -r requirements.txt - python.exe run_tests.py --output_dir test-reports - """ - } - } - stage("Unit Test Results") { - steps { - junit "test-reports/**/*.xml" - } - } - } - - post { - failure { - step([$class: 'Mailer', notifyEveryUnstableBuild: true, recipients: 'icp-buildserver@lists.isis.rl.ac.uk', sendToIndividuals: true]) - } - } - - // The options directive is for configuration that applies to the whole job. - options { - buildDiscarder(logRotator(numToKeepStr:'5', daysToKeepStr: '7')) - timeout(time: 60, unit: 'MINUTES') - disableConcurrentBuilds() - } -} diff --git a/pyproject.toml b/pyproject.toml index 6806c00..06bfb6b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,11 +4,11 @@ build-backend = "setuptools.build_meta" [project] -name = "ExperimentDetailsPopulator" +name = "ExperimentDatabasePopulator" dynamic = ["version"] -description = "Experiment Details Populator" +description = "Experiment Database Populator" readme = "README.md" -requires-python = ">=3.10" +requires-python = ">=3.8" license = {file = "LICENSE"} authors = [ @@ -22,7 +22,7 @@ classifiers = [ "Development Status :: 5 - Production/Stable", "Intended Audience :: Developers", "License :: OSI Approved :: BSD License", - "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3 :: Only", ] dependencies = [ @@ -33,17 +33,14 @@ dependencies = [ "pymysql", "pyepics", "cryptography", - "six", + "mock", # Needed at runtime, not dev-only ] [project.optional-dependencies] - dev = [ "pytest", "pytest-cov", "ruff>=0.9", - "mock", - "xmlrunner", ] [project.urls] diff --git a/rb_number_populator.sh b/rb_number_populator.sh index 79cf580..4febb30 100755 --- a/rb_number_populator.sh +++ b/rb_number_populator.sh @@ -3,5 +3,5 @@ venv="exp_db_populator_venv" # Name of the virtual environment . /home/epics/EPICS/config_env.sh source /home/epics/RB_num_populator/$venv/bin/activate # activate the virtual environment -/home/epics/RB_num_populator/$venv/bin/python3.8 /home/epics/RB_num_populator/main.py +exp_db_populator deactivate # deactivate the virtual environment diff --git a/run_tests.py b/run_tests.py deleted file mode 100644 index a0397f6..0000000 --- a/run_tests.py +++ /dev/null @@ -1,37 +0,0 @@ -import argparse -import os -import sys - -# Standard imports -import unittest - -import xmlrunner - -DEFAULT_DIRECTORY = os.path.join(".", "test-reports") - - -if __name__ == "__main__": - # get output directory from command line arguments - parser = argparse.ArgumentParser() - parser.add_argument( - "-o", - "--output_dir", - nargs=1, - type=str, - default=[DEFAULT_DIRECTORY], - help="The directory to save the test reports", - ) - args = parser.parse_args() - xml_dir = args.output_dir[0] - - # Load tests from test suites - test_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), "tests")) - test_suite = unittest.TestLoader().discover(test_dir, pattern="test_*.py") - - print("\n\n------ BEGINNING Experiment Database Populator UNIT TESTS ------") - ret_vals = list() - ret_vals.append(xmlrunner.XMLTestRunner(output=xml_dir).run(test_suite)) - print("------ UNIT TESTS COMPLETE ------\n\n") - - # Return failure exit code if a test failed - sys.exit(False in ret_vals) From a465dd9f5de4ea09da6aa8f8c64339b1d1879a7c Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 17 Jan 2025 17:53:51 +0000 Subject: [PATCH 08/12] mysql 8.4 caching_sha2_password compat --- exp_db_populator/cli.py | 4 +++- pyproject.toml | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/exp_db_populator/cli.py b/exp_db_populator/cli.py index 734cc36..24252e6 100644 --- a/exp_db_populator/cli.py +++ b/exp_db_populator/cli.py @@ -142,7 +142,9 @@ def main_cli(): main.prev_inst_list = debug_inst_list main.inst_list_changes(debug_inst_list) elif args.test_data: - data = [create_web_data_with_experimenters_and_other_date(TEST_USER_1, datetime.now())] + data = [create_web_data_with_experimenters_and_other_date([TEST_USER_1], datetime.now())] + if not args.db_user or not args.db_pass: + raise ValueError("Must specify a username and password if using test data") update( "localhost", "localhost", diff --git a/pyproject.toml b/pyproject.toml index 06bfb6b..71213dc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -30,9 +30,9 @@ dependencies = [ "suds", "pykeepass", "peewee", - "pymysql", - "pyepics", + "pymysql[rsa]", "cryptography", + "pyepics", "mock", # Needed at runtime, not dev-only ] From e31616c157240f8f7b8aae045690e1cf5b185748 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 17 Jan 2025 17:55:59 +0000 Subject: [PATCH 09/12] Remove debug print --- exp_db_populator/gatherer.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/exp_db_populator/gatherer.py b/exp_db_populator/gatherer.py index eb45719..505a151 100644 --- a/exp_db_populator/gatherer.py +++ b/exp_db_populator/gatherer.py @@ -54,8 +54,6 @@ def run(self): while self.running: all_data = gather_data() - print(all_data) - for inst in self.inst_list: if inst["isScheduled"]: name, host = correct_name(inst["name"]), inst["hostName"] From 463dfa45ee1ec32f21393cc836ff0c025225ddb3 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 17 Jan 2025 17:58:52 +0000 Subject: [PATCH 10/12] Create venv correctly --- create_rb_number_populator_python_venv.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/create_rb_number_populator_python_venv.sh b/create_rb_number_populator_python_venv.sh index 06cd718..06bd5de 100644 --- a/create_rb_number_populator_python_venv.sh +++ b/create_rb_number_populator_python_venv.sh @@ -3,6 +3,6 @@ venv="exp_db_populator_venv" # Name of the virtual environment /usr/local/bin/python3.8 -m venv /home/epics/RB_num_populator/$venv # create virtual environment source $venv/bin/activate # activate the virtual environment -$venv/bin/pip install -r requirements.txt # Install requirements.txt +$venv/bin/pip install -e . # Install requirements.txt deactivate # deactivate the virtual environment echo "Virtual environment created" From 0a96fa2dc8a624dd10ef73f73eb4d837bf7cb5d2 Mon Sep 17 00:00:00 2001 From: Jack Harper Date: Wed, 22 Jan 2025 10:59:38 +0000 Subject: [PATCH 11/12] Update create_rb_number_populator_python_venv.sh --- create_rb_number_populator_python_venv.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/create_rb_number_populator_python_venv.sh b/create_rb_number_populator_python_venv.sh index 06bd5de..1ad8c2d 100644 --- a/create_rb_number_populator_python_venv.sh +++ b/create_rb_number_populator_python_venv.sh @@ -3,6 +3,6 @@ venv="exp_db_populator_venv" # Name of the virtual environment /usr/local/bin/python3.8 -m venv /home/epics/RB_num_populator/$venv # create virtual environment source $venv/bin/activate # activate the virtual environment -$venv/bin/pip install -e . # Install requirements.txt +$venv/bin/pip install -e . # Install requirements deactivate # deactivate the virtual environment echo "Virtual environment created" From e6cb7781778370edf536f41e9a52590f2e99f00b Mon Sep 17 00:00:00 2001 From: Jack Harper Date: Wed, 22 Jan 2025 11:50:55 +0000 Subject: [PATCH 12/12] remove redundant test --- tests/test_populator.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/test_populator.py b/tests/test_populator.py index d0b2d0e..d014c4a 100644 --- a/tests/test_populator.py +++ b/tests/test_populator.py @@ -116,9 +116,6 @@ def test_GIVEN_experiment_and_related_experiment_teams_WHEN_unreferenced_removed remove_experiments_not_referenced() self.assertEqual(1, model.Experiment.select().count()) - def test_WHEN_populate_called_with_experiments_and_no_teams_THEN_exception_raised(self): - self.assertRaises(KeyError, populate, ["TEST"], []) - def create_experiments_dictionary(self): return [ { @@ -140,7 +137,7 @@ def create_experiment_teams_dictionary(self): exp_team_data.user.user_id = 1 return [exp_team_data] - def test_WHEN_populate_called_with_experiments_and_no_teams_2THEN_exception_raised(self): + def test_WHEN_populate_called_with_experiments_and_no_teams_THEN_exception_raised(self): experiments = self.create_experiments_dictionary() self.assertRaises(KeyError, populate, experiments, [])