From cf609eae6b1b3285b34134a4fe2d24e96017da22 Mon Sep 17 00:00:00 2001
From: Luc Didry <luc@didry.org>
Date: Mon, 18 Mar 2024 09:01:39 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=A8=20=E2=80=94=20Improve=20code=20str?=
 =?UTF-8?q?ucture=20following=20review=20of=20MR=20!25?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 Makefile                                      |  2 ++
 .../1a3497f9f71b_adding_configcache_model.py  | 14 +++-----
 argos/server/models.py                        |  8 ++++-
 argos/server/queries.py                       | 36 +++++++++----------
 4 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/Makefile b/Makefile
index f42e098..9f61d37 100644
--- a/Makefile
+++ b/Makefile
@@ -24,6 +24,8 @@ djlint: venv ## Format the templates
 	venv/bin/djlint --ignore=H030,H031,H006 --profile jinja --lint argos/server/templates/*html
 pylint: venv ## Runs pylint on the code
 	venv/bin/pylint argos
+pylint-alembic: venv ## Runs pylint on alembic migration files
+	venv/bin/pylint --disable invalid-name,no-member alembic/versions/*.py
 lint: djlint pylint
 help:
 	@python3 -c "$$PRINT_HELP_PYSCRIPT" < $(MAKEFILE_LIST)
diff --git a/alembic/versions/1a3497f9f71b_adding_configcache_model.py b/alembic/versions/1a3497f9f71b_adding_configcache_model.py
index 2f92fd3..645bdc7 100644
--- a/alembic/versions/1a3497f9f71b_adding_configcache_model.py
+++ b/alembic/versions/1a3497f9f71b_adding_configcache_model.py
@@ -7,8 +7,8 @@ Create Date: 2024-03-13 15:28:09.185377
 """
 from typing import Sequence, Union
 
-from alembic import op
 import sqlalchemy as sa
+from alembic import op
 
 
 # revision identifiers, used by Alembic.
@@ -19,17 +19,13 @@ depends_on: Union[str, Sequence[str], None] = None
 
 
 def upgrade() -> None:
-    # ### commands auto generated by Alembic - please adjust! ###
     op.create_table('config_cache',
-    sa.Column('name', sa.String(), nullable=False),
-    sa.Column('val', sa.String(), nullable=False),
-    sa.Column('updated_at', sa.DateTime(), nullable=False),
-    sa.PrimaryKeyConstraint('name')
+        sa.Column('name', sa.String(), nullable=False),
+        sa.Column('val', sa.String(), nullable=False),
+        sa.Column('updated_at', sa.DateTime(), nullable=False),
+        sa.PrimaryKeyConstraint('name')
     )
-    # ### end Alembic commands ###
 
 
 def downgrade() -> None:
-    # ### commands auto generated by Alembic - please adjust! ###
     op.drop_table('config_cache')
-    # ### end Alembic commands ###
diff --git a/argos/server/models.py b/argos/server/models.py
index 0637e7e..75ed8ab 100644
--- a/argos/server/models.py
+++ b/argos/server/models.py
@@ -117,7 +117,13 @@ class Result(Base):
 class ConfigCache(Base):
     """Contains some informations on the previous config state
 
-    Used to quickly determine if we need to update the tasks
+    Used to quickly determine if we need to update the tasks.
+    There is currently two cached settings:
+    - general_frequency: the content of general.frequency setting, in minutes
+                         ex: 5
+    - websites_hash:     the sha256sum of websites setting, to allow a quick
+                         comparison without looping through all websites
+                         ex: 8b886e7db7b553fe99f6d5437f31745987e243c77b2109b84cf9a7f8bf7d75b1
     """
     __tablename__ = "config_cache"
     name: Mapped[str] = mapped_column(primary_key=True)
diff --git a/argos/server/queries.py b/argos/server/queries.py
index 6d4d790..742c516 100644
--- a/argos/server/queries.py
+++ b/argos/server/queries.py
@@ -62,7 +62,7 @@ async def count_results(db: Session):
     return db.query(Result).count()
 
 
-async def is_config_unchanged(db: Session, config: schemas.Config) -> bool:
+async def has_config_changed(db: Session, config: schemas.Config) -> bool:
     """Check if websites config has changed by using a hashsum and a config cache"""
     websites_hash = sha256(str(config.websites).encode()).hexdigest()
     conf_caches = (
@@ -72,24 +72,24 @@ async def is_config_unchanged(db: Session, config: schemas.Config) -> bool:
     same_config = True
     if conf_caches:
         for conf in conf_caches:
-            if not same_config:
-                break
+            match (conf.name):
+                case 'websites_hash':
+                    if conf.val != websites_hash:
+                        same_config = False
+                        conf.val = websites_hash
+                        conf.updated_at = datetime.now()
+                case 'general_frequency':
+                    if conf.val != str(config.general.frequency):
+                        same_config = False
+                        conf.val = config.general.frequency
+                        conf.updated_at = datetime.now()
 
-            if conf.name == 'websites_hash':
-                same_config = conf.val == websites_hash
-            elif conf.name == 'general_frequency':
-                same_config = conf.val == str(config.general.frequency)
+        db.commit()
 
         if same_config:
-            return True
+            return False
 
-        for conf in conf_caches:
-            if conf.name == 'websites_hash':
-                conf.val = websites_hash
-            elif conf.name == 'general_frequency':
-                conf.val = config.general.frequency
-            conf.updated_at = datetime.now()
-    else:
+    else:  # no config cache found
         web_hash = ConfigCache(
                 name='websites_hash',
                 val=websites_hash,
@@ -104,13 +104,13 @@ async def is_config_unchanged(db: Session, config: schemas.Config) -> bool:
         db.add(gen_freq)
         db.commit()
 
-    return False
+    return True
 
 
 async def update_from_config(db: Session, config: schemas.Config):
     """Update tasks from config file"""
-    config_unchanged = await is_config_unchanged(db, config)
-    if config_unchanged:
+    config_changed = await has_config_changed(db, config)
+    if not config_changed:
         return {'added': 0, 'vanished': 0}
 
     max_task_id = (