WIP: review the overall structure

.flake8

@@ -1,22 +1,21 @@
 # ********************************
 # |docname| - Flake8 configuration
 # ********************************
-# Docs todo: provide nice hyperlinks to docs on these configuration settings, and perhaps some rationale (why 88 characters, etc.).
+# To run, execute ``flake8`` from the root directory of the project.
 #
-# To run, execute ``flake8 .`` from the root directory of the project.
 [flake8]
+# Use `Black's default <https://black.readthedocs.io/en/stable/compatible_configs.html#flake8>`_ of 88 charaters per line.
 max-line-length=88
-ignore=F821,
+ignore=
+  # To be compatible with `Black's default`_.
   W503,
-  # space before ``:``
   E203,
-  E501,
+  # Block comment should start with ``#``. See `pycodestyle error codes <https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes>`_. We use this to comment code out.
   E265,
-  # too many ``#``
+  # Too many leading '#' for block comment. Again, for commenting code out.
   E266,
-  E711,
-  # web2py needs to compare to ``== True`` or ``== False`` for queries.
-  E712
+  # Line too long (82 > 79 characters). Flake8 complains about comment lines being too long, while Black allows tihs. Disable flake8's long line detection to avoid these spurious warnings.
+  E501,
 
 exclude =
   # Ignore Sphinx build output.

.github/workflows/python-package.yml

@@ -30,12 +30,9 @@ jobs:
       run: |
         pip install -U pip
         pip install poetry
+        # Why are we doing an update here? Updates should be performed by committers, not automatically.
         poetry update
         poetry install
-    - name: flake8 quick test
-      run: |
-        # stop the build if there are Python syntax errors or undefined names
-        poetry run flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
     - name: Run the Pre Commit Check Script
       run: |
         poetry run ./pre_commit_check.py

README.rst

@@ -0,0 +1,34 @@
+*******************************************
+New FastAPI-based Book Server for Runestone
+*******************************************
+
+The goal of this project is to replace the parts of the web2py-based RunestoneServer.
+
+We would love development help on this.  Please see our docs including information on contributing to this project `on readthedocs <https://bookserver.readthedocs.io/en/latest/>`_.
+
+
+Quickstart
+==========
+First, install BookServer.
+
+Installation options
+--------------------
+For development:
+
+-   Clone this repository.
+-   `Install poetry <https://python-poetry.org/docs/#installation>`_.
+-   From the command line / terminal, change to the directory containing this repo then execute ``poetry install``.
+
+To install from PyPi:
+-   From the command line / terminal, execute ``python -m pip install -U BookServer`` or ``python3 -m pip install -U BookServer``.
+
+Building books
+--------------
+-   Check out the ``bookserver`` branch of the Ruestone Components repo and install it.
+-   Build a book with this branch.
+-   Copy it the `book path <book_path>` or update the book path to point to the location of a built book.
+-   Add the book's info to the database.
+
+Running the server
+------------------
+From the command line / terminal, execute ``poetry run uvicorn bookserver.main:app --reload --port 8080``. If running in development mode, this must be executed from the directory containing the repo.

alembic/env.py

@@ -1,6 +1,6 @@
-# ********************************
-# |docname|- Alembic configuration
-# ********************************
+# *********************************
+# |docname| - Alembic configuration
+# *********************************
 # :index:`docs to write`: Better description...
 #
 # Imports

bookserver/__init__.py

@@ -1,3 +0,0 @@
-# ********************************************
-# |docname| - Declare this directory a package
-# ********************************************

bookserver/__main__.py

@@ -0,0 +1,10 @@
+# *****************************************************
+# |docname| - Provide a simple method to run the server
+# *****************************************************
+# From the terminal / command line, execute ``python -m bookserver``, which causes this script to run.
+
+if __name__ == "__main__":
+    import uvicorn
+
+    # See https://www.uvicorn.org/deployment/#running-programmatically.
+    uvicorn.run("bookserver.main:app", port=8080)

bookserver/applogger.py

@@ -11,7 +11,6 @@
 import logging
 import sys
 
-#
 # Third-party imports
 # -------------------
 # None.

bookserver/config.py

@@ -37,25 +37,24 @@ class Settings(BaseSettings):
 
     google_ga: str = ""
 
-    # Either ``development``, ``production``, or ``test``, per `this code <setting.dev_dburl>`.
-    config: str = "development"  # production or test
+    # Either ``development``, ``production``, or ``test``, per `this code <setting.dev_dburl>`. TODO: Use an Enum for this instead! (Will that work with env vars?)
+    config: str = "development"
 
-    # `Database setup <setting.dev_dburl>`.
+    # `Database setup <setting.dev_dburl>`. It must be an async connection; for example:
+    #
+    # - ``sqlite+aiosqlite:///./runestone.db``
+    # - ``postgresql+asyncpg://postgres:bully@localhost/runestone``
     prod_dburl: str = "sqlite+aiosqlite:///./runestone.db"
     dev_dburl: str = "sqlite+aiosqlite:///./runestone_dev.db"
     test_dburl: str = "sqlite+aiosqlite:///./runestone_test.db"
 
-    # Configure ads.
+    # Configure ads. TODO: Link to the place in the Runestone Components where this is used.
     adsenseid: str = ""
     num_banners: int = 0
     serve_ad: bool = False
 
-    # :index:`docs to write`: **What's this?**
-    library_path: str = "/Users/bmiller/Runestone"
-    dbserver: str = "sqlite"
-
-    # Specify the directory to serve books from.
-    book_path: Path = Path(__file__).parents[1] / "books"
+    # _`book_path`: specify the directory to serve books from.
+    book_path: Path = Path.home() / "Runestone/books"
 
     # This is the secret key used for generating the JWT token
     secret: str = "supersecret"

bookserver/db.py

@@ -13,7 +13,7 @@
 #
 # Third-party imports
 # -------------------
-# Enable asyncio for SQLAlchemy -- see `databases <https://www.encode.io/databases/>`_.
+# Use asyncio for SQLAlchemy -- see `SQLAlchemy Asynchronous I/O (asyncio) <https://docs.sqlalchemy.org/en/14/orm/extensions/asyncio.html>`_.
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.ext.asyncio import AsyncSession
 from sqlalchemy.ext.asyncio import create_async_engine
@@ -25,11 +25,6 @@
 # See `./config.py`.
 from bookserver.config import settings
 
-# :index:`question`: does this belong in `./config.py`?  Or does it just describe the format of a database URL for two databases?
-#
-## SQLALCHEMY_DATABASE_URL = "sqlite:///./bookserver.db"
-## SQLALCHEMY_DATABASE_URL = "postgresql://user:password@postgresserver/db"
-
 # .. _setting.dev_dburl:
 if settings.config == "development":
     DATABASE_URL = settings.dev_dburl
@@ -40,11 +35,12 @@
 else:
     assert False
 
-if settings.dbserver == "sqlite":
+if DATABASE_URL.startswith("sqlite"):
     connect_args = {"check_same_thread": False}
 else:
     connect_args = {}
 
+# TODO: Remove the ``echo=True`` when done debugging.
 engine = create_async_engine(DATABASE_URL, connect_args=connect_args, echo=True)
 # This creates the SessionLocal class.  An actual session is an instance of this class.
 async_session = sessionmaker(engine, class_=AsyncSession, expire_on_commit=False)

bookserver/internal/utils.py

@@ -31,8 +31,8 @@ def canonicalize_tz(tstring: str) -> str:
     """
     x = re.search(r"\((.*)\)", tstring)
     if x:
-        x = x.group(1)
-        y = x.split()
+        z = x.group(1)
+        y = z.split()
         if len(y) == 1:
             return tstring
         else:

bookserver/main.py

@@ -25,6 +25,7 @@
 from .routers import rslogging
 from .db import init_models
 from .session import auth_manager
+from .config import settings
 from bookserver.applogger import rslogger
 
 # FastAPI setup
@@ -33,6 +34,7 @@
 # Base.metadata.create_all()
 
 app = FastAPI()
+print(f"Serving books from {settings.book_path}.\n")
 
 # Routing
 # -------
@@ -54,9 +56,9 @@ async def startup():
     await init_models()
 
 
-# @app.on_event("shutdown")
-# async def shutdown():
-#     await database.disconnect()
+## @app.on_event("shutdown")
+## async def shutdown():
+##     await database.disconnect()
 
 
 # this is just a simple example of adding a middleware
@@ -101,9 +103,3 @@ def auth_exception_handler(request: Request, exc: NotAuthenticatedException):
     Redirect the user to the login page if not logged in
     """
     return RedirectResponse(url="/auth/login")
-
-
-if __name__ == "__main__":
-    import uvicorn
-
-    uvicorn.run(app, host="0.0.0.0", port="8080")

bookserver/models.py

@@ -1,3 +1,4 @@
+# *****************************************
 # |docname| - definition of database models
 # *****************************************
 # In this file we define our SQLAlchemy data models. These get translated into relational database tables.
@@ -41,7 +42,6 @@
 
 # Local application imports
 # -------------------------
-# None.
 from .db import Base
 
 
@@ -99,12 +99,11 @@ class IdMixin:
 
 # Useinfo
 # -------
-# This defines the useinfo table in the database.  This table logs nearly every click
-# generated by a student.  It gets very large and needs a lot of indexes to keep Runestone
+# This defines the useinfo table in the database. This table logs nearly every click
+# generated by a student. It gets very large and needs a lot of indexes to keep Runestone
 # from bogging down.
-# Useinfo
-# -------
-# User info logged by the `hsblog endpoint`. See there for more info.
+#
+# User info logged by the `log_book_event endpoint`. See there for more info.
 class Useinfo(Base, IdMixin):
     __tablename__ = "useinfo"
     # _`timestamp`: when this entry was recorded by this webapp.
@@ -318,11 +317,11 @@ class Courses(Base, IdMixin):
     downloads_enabled = Column(Web2PyBoolean)
     courselevel = Column(String)
 
-    # # Create ``child_courses`` which all refer to a single ``parent_course``: children's ``base_course`` matches a parent's ``course_name``. See `adjacency list relationships <http://docs.sqlalchemy.org/en/latest/orm/self_referential.html#self-referential>`_.
-    # child_courses = relationship(
-    #
-    #     "Courses", backref=backref("parent_course", remote_side=[course_name])
-    # )
+    ## # Create ``child_courses`` which all refer to a single ``parent_course``: children's ``base_course`` matches a parent's ``course_name``. See `adjacency list relationships <http://docs.sqlalchemy.org/en/latest/orm/self_referential.html#self-referential>`_.
+    ## child_courses = relationship(
+    ##
+    ##     "Courses", backref=backref("parent_course", remote_side=[course_name])
+    ## )
 
     # Define a default query: the username if provided a string. Otherwise, automatically fall back to the id.
     @classmethod

bookserver/routers/assessment.py

@@ -15,19 +15,16 @@
 #
 # Standard library
 # ----------------
-import datetime
+# None.
 
 # Third-party imports
 # -------------------
-# :index:`todo`: **Lots of unused imports here...**
-from dateutil.parser import parse
 from fastapi import APIRouter
 
 # Local application imports
 # -------------------------
 from ..applogger import rslogger
 from ..crud import create_useinfo_entry, fetch_last_answer_table_entry  # noqa F401
-from ..internal.utils import canonicalize_tz
 from ..schemas import AssessmentRequest, LogItem, LogItemIncoming  # noqa F401
 
 # Routing
@@ -50,19 +47,6 @@ async def get_assessment_results(request_data: AssessmentRequest):
     # else:
     #     sid = auth.user.username
 
-    # :index:`todo`: **This whole thing is messy - get the deadline from the assignment in the db.**
-    if request_data.deadline:
-        try:
-            deadline = parse(canonicalize_tz(request_data.deadline))
-            tzoff = session.timezoneoffset if session.timezoneoffset else 0
-            deadline = deadline + datetime.timedelta(hours=float(tzoff))
-            deadline = deadline.replace(tzinfo=None)
-        except Exception:
-            rslogger.error(f"Bad Timezone - {request_data.deadline}")
-            deadline = datetime.datetime.utcnow()
-    else:
-        request_data.deadline = datetime.datetime.utcnow()
-
     # Identify the correct event and query the database so we can load it from the server
 
     row = await fetch_last_answer_table_entry(request_data)

bookserver/routers/auth.py

@@ -19,7 +19,6 @@
 #
 # Third-party imports
 # -------------------
-# :index:`todo`: **Lots of unused imports...can we deletet these?**
 from fastapi import APIRouter, Depends, Request, Response  # noqa F401
 from fastapi_login.exceptions import InvalidCredentialsException
 from fastapi.security import OAuth2PasswordRequestForm
@@ -46,6 +45,10 @@
 templates = Jinja2Templates(directory=f"bookserver/templates{router.prefix}")
 
 
+# .. _login:
+#
+# login
+# -----
 @router.get("/login", response_class=HTMLResponse)
 def login_form(request: Request):
     return templates.TemplateResponse("login.html", {"request": request})

bookserver/routers/books.py

@@ -14,8 +14,6 @@
 
 # Third-party imports
 # -------------------
-# :index:`todo`: **Lots of unused imports here...can we remove them?***
-
 from fastapi import APIRouter, Depends, Request, HTTPException  # noqa F401
 from fastapi.responses import FileResponse, HTMLResponse
 from fastapi.templating import Jinja2Templates
@@ -118,6 +116,7 @@ async def serve_page(
         course_name=course,
         base_course=course,
         user_id=user.username,
+        # TODO
         user_email="bonelake@mac.com",
         downloads_enabled="false",
         allow_pairs="false",

bookserver/routers/rslogging.py

@@ -14,7 +14,6 @@
 #
 # Third-party imports
 # -------------------
-# :index:`todo`: **Lots of unused imports...can we deletet these?**
 from fastapi import APIRouter, Depends  # noqa F401
 
 # Local application imports
@@ -32,8 +31,12 @@
 )
 
 
+# .. _log_book_event endpoint:
+#
+# log_book_event endpoint
+# -----------------------
 @router.post("/bookevent")
-async def log_book_event(entry: LogItemIncoming):
+async def log_book_event(entry: LogItem):
     """
     This endpoint is called to log information for nearly every click that happens in the textbook.
     It uses the ``LogItem`` object to define the JSON payload it gets from a page of a book.