Add lint tools to the project

Added two Makefile recipes in order to format the code and pass the
linter and the type checker using autopep8 for formatting and mypy and
flake for linting.

This will ensure that all contributions to this project will follow the
same rules and the code will be more traceble and easier to read and
follow.

As part of this PR there are some other minor fixes:

* Removed `*` imports
* Fixed some naming conventions
* Fixed some Imports between SqlModel/SqlAlchemy objects
* Added type returns to all the functions
* Removed unused imports
* Fix minor style issues

Signed-off-by: Oscar Moya <[email protected]>

.flake8

@@ -0,0 +1,18 @@
+[flake8]
+max-line-length = 120
+exclude = .git,
+    __pycache__,
+    docs/source/conf.py,
+    old,build,
+    dist,
+    venv,
+    .venv,
+    .tox,
+    .eggs,
+    *.egg,
+    *.egg-info,
+    *.egg-info/*
+    __init__.py
+ignore = E203, E266, W503
+max-complexity = 18
+select = B,C,E,F,W,T4,B9

.github/workflows/ci.yaml

@@ -12,6 +12,54 @@ env:
   PLATFORMS: linux/amd64,linux/arm64
 
 jobs:
+  lint: 
+    runs-on: ubuntu-22.04
+    steps:
+      - name: "Checkout backend(REST)"
+        uses: actions/checkout@v4
+        with:
+          path: backend
+          fetch-depth: 0
+          fetch-tags: 1
+
+      - name: "lint"
+        run: |
+          cd backend
+
+          echo "Fix mess with tags in actions/checkout..."
+          git fetch -f && git fetch -f --tags
+
+          sudo apt-get install -y make
+          pip3 install -r requirements.txt
+          pip3 install -r requirements-dev.txt
+
+          #Run linter
+          make lint
+  
+  format:
+    runs-on: ubuntu-22.04
+    steps:
+      - name: "Checkout backend(REST)"
+        uses: actions/checkout@v4
+        with:
+          path: backend
+          fetch-depth: 0
+          fetch-tags: 1
+
+      - name: "format"
+        run: |
+          cd backend
+
+          echo "Fix mess with tags in actions/checkout..."
+          git fetch -f && git fetch -f --tags
+
+          sudo apt-get install -y make
+          pip3 install -r requirements.txt
+          pip3 install -r requirements-dev.txt
+
+          #Run formatter
+          AUTOPEP8_EXIT_CODE=--exit-code make format-code
+
   unit-test:
     runs-on: ubuntu-22.04
     steps:
@@ -37,7 +85,7 @@ jobs:
 
   docker_build_and_publish:
     runs-on: ubuntu-22.04
-    needs: [unit-test]
+    needs: [unit-test, format, lint]
     steps:
       - name: "Checkout backend(REST)"
         uses: actions/checkout@v4

.mypy.ini

@@ -0,0 +1,4 @@
+[mypy]
+check_untyped_defs = True
+disallow_untyped_calls = True
+ignore_missing_imports = True

Makefile

@@ -1,3 +1,6 @@
+AUTOPEP8_OPTS ?= --in-place --recursive --aggressive --aggressive
+AUTOPEP8_EXIT_CODE ?=
+
 all: start
 
 start: _build _run
@@ -10,3 +13,8 @@ stop:
 	@docker kill rest_backend || true
 	@docker rm rest_backend || true
 reload: stop start
+format-code:
+	autopep8 $(AUTOPEP8_OPTS) $(AUTOPEP8_EXIT_CODE) ./
+lint:
+	flake8 ./
+	mypy ./

requirements-dev.txt

@@ -0,0 +1,3 @@
+mypy
+flake8
+autopep8

requirements.txt

@@ -6,3 +6,4 @@ psycopg2==2.9.9
 jinja2==3.1.3
 pyyaml==6.0.1
 pytest
+types-requests

src/constants.py

@@ -1,17 +1,17 @@
-#DB
+# DB
 
-##Driver
-DRIVER="sqlite"
+# Driver
+DRIVER = "sqlite"
 
-##SQLite
-SQLITE_FILE="./data/backend.db"
+# SQLite
+SQLITE_FILE = "./data/backend.db"
 
-##Postgres
-POSTGRES_HOST="127.0.0.1"
-POSTGRES_PORT=5432
-POSTGRES_DB="stack"
-POSTGRES_USERNAME="postgres"
-POSTGRES_PASSWORD="test"
+# Postgres
+POSTGRES_HOST = "127.0.0.1"
+POSTGRES_PORT = 5432
+POSTGRES_DB = "stack"
+POSTGRES_USERNAME = "postgres"
+POSTGRES_PASSWORD = "test"
 
-#API
-URL_PATH="/rest"
+# API
+URL_PATH = "/rest"

src/data/database.py

@@ -1,22 +1,36 @@
+from typing import Any, Generator
 from sqlmodel import SQLModel, create_engine, Session
 from sqlalchemy_utils import database_exists, create_database
-from constants import *
+import constants
 
-def createTables(db_engine):
+
+def create_tables(db_engine) -> None:
     from models.turnilo_dashboard import TurniloDashboard
-    SQLModel.metadata.create_all(db_engine)
+    SQLModel.metadata.create_all(db_engine, tables=[TurniloDashboard.__table__])
+
 
-if DRIVER == "sqlite":
-    engine = create_engine("sqlite:///"+SQLITE_FILE, connect_args={"check_same_thread": False})
+if constants.DRIVER == "sqlite":
+    engine = create_engine("sqlite:///" + constants.SQLITE_FILE, connect_args={"check_same_thread": False})
 else:
-    engine = create_engine("postgresql://"+POSTGRES_USERNAME+":"+POSTGRES_PASSWORD+"@"+POSTGRES_HOST+":"+str(POSTGRES_PORT)+"/"+POSTGRES_DB)
+    engine = create_engine(
+        "postgresql://"
+        + constants.POSTGRES_USERNAME
+        + ":"
+        + constants.POSTGRES_PASSWORD
+        + "@"
+        + constants.POSTGRES_HOST
+        + ":"
+        + str(constants.POSTGRES_PORT)
+        + "/"
+        + constants.POSTGRES_DB)
 
 if not database_exists(engine.url):
     print("Creating DB...")
-    create_database(engine.url)
-    createTables(engine)
+    create_database(engine.url)  # type: ignore
+    create_tables(engine)
+
 
-def get_session():
+def get_session() -> Generator[Session, Any, None]:
     db_session = Session(engine)
     try:
         yield db_session

src/models/turnilo_dashboard.py

@@ -1,8 +1,9 @@
 from typing import Optional
-from sqlmodel import Field, Session, SQLModel, UniqueConstraint
+from sqlmodel import Field, SQLModel, UniqueConstraint
+
 
 class TurniloDashboard(SQLModel, table=True):
-    __tablename__ = "turniloDashboards"
+    __tablename__ = "turniloDashboards"  # type: ignore
     __table_args__ = (UniqueConstraint('dataCube', 'shortName', name='_dataCube_shortName'),)
 
     id: Optional[int] = Field(default=None, primary_key=True, index=True)

src/routes/routes.py

@@ -1,70 +1,77 @@
-from fastapi import APIRouter, Depends, HTTPException, status, Path
+from fastapi import APIRouter, Depends, HTTPException
 from typing import List
 from sqlalchemy.orm import Session
 from models.turnilo_dashboard import TurniloDashboard
-from services.turnilo_dashboards import *
+from services import turnilo_dashboards as td
 
-from constants import *
+import constants
 import data.database as db
 
 api_router = APIRouter()
 
 ### Dashboards ###
 
-#GET
+# GET
+
+
 @api_router.get(
-    URL_PATH+"/turnilo/dashboards/",
+    constants.URL_PATH + "/turnilo/dashboards/",
     response_model=List[TurniloDashboard],
     summary="Gets all Turnilo dashboards"
 )
 def turnilo_get_dashboards(db_session: Session = Depends(db.get_session)):
-    return dashboards_get_all(db_session)
+    return td.dashboards_get_all(db_session)
+
 
 @api_router.get(
-    URL_PATH+"/turnilo/dashboards/{id}",
+    constants.URL_PATH + "/turnilo/dashboards/{id}",
     response_model=TurniloDashboard,
     summary="Get a Turnilo dashboard by id (integer)"
 )
-def turnilo_get_dashboard_id(id:str, db_session: Session = Depends(db.get_session)):
+def turnilo_get_dashboard_id(id: str, db_session: Session = Depends(db.get_session)):
     try:
         int_id = int(id)
-    except:
+    except BaseException:
         raise HTTPException(status_code=400, detail="Id is not an integer")
-    return dashboards_get_id(db_session, int_id)
+    return td.dashboards_get_id(db_session, int_id)
+
+# POST
+
 
-#POST
 @api_router.post(
-    URL_PATH+"/turnilo/dashboards/",
+    constants.URL_PATH + "/turnilo/dashboards/",
     response_model=TurniloDashboard,
     summary="Create a Turnilo dashboard. A unique id will be assigned."
 )
 def turnilo_create_dashboard(dashboard: TurniloDashboard, db_session: Session = Depends(db.get_session)):
-    return dashboards_create(db_session, dashboard)
+    return td.dashboards_create(db_session, dashboard)
 
 
-#PUT
+# PUT
 @api_router.put(
-    URL_PATH+"/turnilo/dashboards/{id}",
+    constants.URL_PATH + "/turnilo/dashboards/{id}",
     response_model=TurniloDashboard,
     summary="Update/replace a Turnilo dashboard. The dashboard (id) must exist"
 )
-def turnilo_update_dashboard(id:str, dashboard: TurniloDashboard, db_session: Session = Depends(db.get_session)):
+def turnilo_update_dashboard(id: str, dashboard: TurniloDashboard, db_session: Session = Depends(db.get_session)):
     try:
         int_id = int(id)
         dashboard.id = int_id
-    except:
+    except BaseException:
         raise HTTPException(status_code=400, detail="Id is not an integer")
-    return dashboards_update(db_session, dashboard)
+    return td.dashboards_update(db_session, dashboard)
+
+# DELETE
+
 
-#DELETE
 @api_router.delete(
-    URL_PATH+"/turnilo/dashboards/{id}",
+    constants.URL_PATH + "/turnilo/dashboards/{id}",
     response_model=TurniloDashboard,
     summary="Delete a Turnilo dashboard"
 )
 def turnilo_delete_dashboard(id: str, db_session: Session = Depends(db.get_session)):
     try:
         int_id = int(id)
-    except:
+    except BaseException:
         raise HTTPException(status_code=400, detail="Id is not an integer")
-    return dashboards_delete(db_session, id)
+    return td.dashboards_delete(db_session, int_id)

src/services/turnilo_dashboards.py

@@ -1,21 +1,30 @@
-from sqlmodel import Session
+from typing import List
+from sqlalchemy.orm import Session
 from sqlalchemy import exc
 from models.turnilo_dashboard import TurniloDashboard
 from fastapi import HTTPException
 
-#Turnilo Dashboards
-def dashboards_get_all(session: Session):
+# Turnilo Dashboards
+
+
+def dashboards_get_all(session: Session) -> List[TurniloDashboard]:
     return session.query(TurniloDashboard).all()
-def _dashboards_return_single_obj(results):
-    if results == None or len(results) == 0:
+
+
+def _dashboards_return_single_obj(results: List[TurniloDashboard]):
+    if results is None or len(results) == 0:
         raise HTTPException(status_code=404, detail="Item not found")
     elif len(results) > 1:
         raise HTTPException(status_code=500, detail="Corrupted state. Get query returned > 1 result")
     return results[0]
-def dashboards_get_id(session: Session, _id: int):
-    results = session.query(TurniloDashboard).filter(TurniloDashboard.id == _id).all()
+
+
+def dashboards_get_id(session: Session, _id: int) -> TurniloDashboard:
+    results: List[TurniloDashboard] = session.query(TurniloDashboard).filter_by(id=_id).all()
     return _dashboards_return_single_obj(results)
-def dashboards_create(session: Session, dashboard: TurniloDashboard):
+
+
+def dashboards_create(session: Session, dashboard: TurniloDashboard) -> TurniloDashboard:
     if dashboard.id:
         raise HTTPException(status_code=400, detail="'id' must NOT be set")
     if not dashboard.shortName or dashboard.shortName == "":
@@ -26,34 +35,37 @@ def dashboards_create(session: Session, dashboard: TurniloDashboard):
     except exc.IntegrityError as e:
         print(str(e))
         raise HTTPException(status_code=400, detail="Integrity error: duplicated datacube+shortName")
-    print("Created dashboard: "+str(dashboard.id))
+    print("Created dashboard: " + str(dashboard.id))
     print(dashboard)
     return dashboard
-def dashboards_update(session: Session, dashboard: TurniloDashboard):
+
+
+def dashboards_update(session: Session, dashboard: TurniloDashboard) -> TurniloDashboard:
     if not dashboard.id:
         raise HTTPException(status_code=400, detail="'id' MUST be set")
     if not dashboard.shortName or dashboard.shortName == "":
         raise HTTPException(status_code=400, detail="shortName not present or empty")
 
-    #First check if it exists
+    # First check if it exists
     dashboards_get_id(session, dashboard.id)
     try:
         session.merge(dashboard)
         session.commit()
     except exc.IntegrityError as e:
         print(str(e))
         raise HTTPException(status_code=400, detail="Integrity error: duplicated datacube+shortName")
-    print("Updated dashboard:"+str(dashboard.id))
+    print("Updated dashboard:" + str(dashboard.id))
     print(dashboard)
     return dashboard
 
-def dashboards_delete(session: Session, _id: int):
-    dashboard=None
+
+def dashboards_delete(session: Session, _id: int) -> TurniloDashboard:
+    dashboard = None
     try:
         dashboard = session.query(TurniloDashboard).filter_by(id=_id).one()
-    except:
+    except BaseException:
         pass
-    if dashboard == None:
+    if dashboard is None:
         raise HTTPException(status_code=404, detail="Item not found")
     session.delete(dashboard)
     session.commit()