Merge pull request #182 from canonical/fix-bg-tasks

Fix bg tasks

Author: muhammad-ali-pk

.dockerignore

@@ -0,0 +1,21 @@
+repositories
+tree-cache
+.venv
+venv/
+__pycache__/
+*.so
+build/
+dist/
+.eggs/
+wheels/
+*.egg-info/
+.installed.cfg
+*.egg
+.pytest_cache/
+.coverage
+htmlcov/
+.tox/
+.coverage.*
+.cache/
+Dockerfile
+docker-compose.yml

.github/workflows/CI.yml

@@ -93,9 +93,51 @@ jobs:
   run-python:
     name: Run Python
     runs-on: ubuntu-latest
+    services:
+      postgres:
+        image: postgres:latest
+        env:
+          POSTGRES_PASSWORD: postgres
+        ports:
+          - 5432:5432
+    steps:
+      - uses: actions/checkout@v3
+      - name: Set up Python 3.12
+        uses: actions/setup-python@v5
+        with:
+          python-version: "3.12"
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          pip install -r requirements.txt
+      - name: Run service
+        env:
+          GOOGLE_PRIVATE_KEY: ${{ secrets.PRIVATE_KEY }}
+          GOOGLE_PRIVATE_KEY_ID: ${{ secrets.PRIVATE_KEY_ID }}
+          DATABASE_URL: postgresql://postgres:postgres@localhost:5432/postgres
+          SECRET_KEY: secret_key
+          GH_TOKEN: ${{ github.token }}
+          REPO_ORG: https://github.com/canonical
+          JIRA_EMAIL: [email protected]
+          JIRA_TOKEN: jiratoken
+          JIRA_URL: https://example.atlassian.net
+          JIRA_LABELS: somelabel
+          JIRA_COPY_UPDATES_EPIC: WD-9999999
+          GOOGLE_DRIVE_FOLDER_ID: folderid
+          COPYDOC_TEMPLATE_ID: templateid
+          FLASK_DEBUG: 1
+        run: |
+          gunicorn webapp.app:app --workers=2 --bind 0.0.0.0:8104 & sleep 3
+          curl --head --fail --retry-delay 1 --retry 30 --retry-connrefused http://localhost:8104
+
+  run-python-redis:
+    name: Run Python Redis
+    runs-on: ubuntu-latest
     services:
       redis:
         image: redis
+        ports:
+          - 6379:6379
         options: >-
           --health-cmd "redis-cli ping"
           --health-interval 10s
@@ -134,9 +176,10 @@ jobs:
           JIRA_COPY_UPDATES_EPIC: WD-9999999
           GOOGLE_DRIVE_FOLDER_ID: folderid
           COPYDOC_TEMPLATE_ID: templateid
+          FLASK_DEBUG: 1
         run: |
-          gunicorn webapp.app:app --daemon --bind 0.0.0.0:8104
-          curl --head --fail --retry-delay 1 --retry 30 --retry-connrefused http://localhost:8104
+          flask --app webapp.app run --debug & sleep 3
+          curl --head --fail --retry-delay 1 --retry 30 --retry-connrefused http://localhost:5000
 
   run-image:
     name: Run Image
@@ -170,11 +213,11 @@ jobs:
           GOOGLE_PRIVATE_KEY_ID: ${{ secrets.PRIVATE_KEY_ID }}
         run: |
           docker run \
-          -p 8104:8104 \
+          -p 8104:80 \
           -e SECRET_KEY=secret_key \
           -e REDIS_HOST=localhost \
           -e REDIS_PORT=6379 \
-          -e GH_TOKEN=${{ github.token }} \
+          -e GH_TOKEN=github.token \
           -e REPO_ORG=https://github.com/canonical \
           -e DATABASE_URL=postgresql://postgres:postgres@localhost:5432/postgres \
           -e [email protected] \

Dockerfile

@@ -32,5 +32,6 @@ ARG BUILD_ID
 ENV TALISKER_REVISION_ID="${BUILD_ID}"
 
 # Setup commands to run web service
+RUN chmod +x ./entrypoint
 ENTRYPOINT ["./entrypoint"]
 CMD ["0.0.0.0:80"]

README.md

@@ -201,7 +201,7 @@ If you do not want to use a dedicated cache, a simple filecache has been include
 
 ```bash
 docker run -d -p 5432:5432 -e POSTGRES_PASSWORD=postgres postgres
-docker run -d -p 6379:6379 valkey/valkey
+docker run -d -p 6379:6379 redis
 ```
 
 #### Virtual Environment
@@ -253,7 +253,7 @@ Please note, make sure the containers for postgres and valkey are already runnin
 
 ```bash
 docker run -d -p 5432:5432 -e POSTGRES_PASSWORD=postgres postgres
-docker run -d -p 6379:6379 valkey/valkey
+docker run -d -p 6379:6379 redis
 ```
 
 You can optionally use dotrun to start the service. When the 1.1.0-rc1 branch is merged, then we can use dotrun without the `--release` flag.

app.py

@@ -1 +1 @@
-from webapp.app import app  # noqa F401
+from webapp.app import app  # noqa: F401

entrypoint

@@ -23,8 +23,5 @@ activate() {
         RUN_COMMAND="${RUN_COMMAND} --reload --log-level debug --timeout 9999"
     fi
 
-    # Run new migrations if needed
-    flask db upgrade
-
     ${RUN_COMMAND}
 }

webapp/__init__.py

@@ -7,9 +7,9 @@
 from werkzeug.middleware.proxy_fix import ProxyFix
 
 from webapp.cache import init_cache
-from webapp.celery import init_celery
 from webapp.context import RegexConverter, base_context, clear_trailing_slash
 from webapp.gdrive import init_gdrive
+from webapp.github import init_github
 from webapp.jira import init_jira
 from webapp.models import init_db
 from webapp.sso import init_sso
@@ -47,8 +47,8 @@ def create_app():
     # Initialize gdrive
     init_gdrive(app)
 
-    # Initialize celery
-    init_celery(app)
+    # Initialize github
+    init_github(app)
 
     return app
 
@@ -67,7 +67,8 @@ def set_security_headers(response):
     if flask.request.endpoint in flask.current_app.view_functions:
         view_func = flask.current_app.view_functions[flask.request.endpoint]
         add_xframe_options_header = not hasattr(
-            view_func, "_exclude_xframe_options_header"
+            view_func,
+            "_exclude_xframe_options_header",
         )
 
     if add_xframe_options_header and "X-Frame-Options" not in response.headers:
@@ -106,10 +107,14 @@ def set_cache_control_headers(response):
 
         max_age = response.cache_control.max_age
         stale_while_revalidate = response.cache_control._get_cache_value(
-            "stale-while-revalidate", False, int
+            "stale-while-revalidate",
+            False,
+            int,
         )
         stale_if_error = response.cache_control._get_cache_value(
-            "stale-if-error", False, int
+            "stale-if-error",
+            False,
+            int,
         )
 
         if type(max_age) is not int:
@@ -142,7 +147,9 @@ def set_cache_control_headers(response):
             #
             # An additional day will hopefully be long enough for most cases.
             response.cache_control._set_cache_value(
-                "stale-while-revalidate", "86400", int
+                "stale-while-revalidate",
+                "86400",
+                int,
             )
 
         if type(stale_if_error) is not int:
@@ -164,7 +171,9 @@ def set_cache_control_headers(response):
             #
             # So we set this to 5 minutes following expiry as a trade-off.
             response.cache_control._set_cache_value(
-                "stale-if-error", "300", int
+                "stale-if-error",
+                "300",
+                int,
             )
 
     return response
@@ -267,7 +276,8 @@ def __init__(
             def not_found_error(error):
                 return (
                     flask.render_template(
-                        template_404, message=error.description
+                        template_404,
+                        message=error.description,
                     ),
                     404,
                 )
@@ -278,7 +288,8 @@ def not_found_error(error):
             def internal_error(error):
                 return (
                     flask.render_template(
-                        template_500, message=error.description
+                        template_500,
+                        message=error.description,
                     ),
                     500,
                 )
@@ -294,7 +305,8 @@ def status_check():
             @self.route("/favicon.ico")
             def favicon():
                 return flask.send_file(
-                    favicon_path, mimetype="image/vnd.microsoft.icon"
+                    favicon_path,
+                    mimetype="image/vnd.microsoft.icon",
                 )
 
         elif favicon_url:

webapp/app.py

@@ -3,17 +3,22 @@
 from flask import render_template
 
 from webapp import create_app
-from webapp.github import init_github
+from webapp.celery import init_celery
 from webapp.routes.jira import jira_blueprint
 from webapp.routes.product import product_blueprint
 from webapp.routes.tree import tree_blueprint
 from webapp.routes.user import user_blueprint
+from webapp.scheduled_tasks import init_scheduled_tasks
 from webapp.sso import login_required
 
 app = create_app()
 
-# Tasks
-init_github(app)
+# Initialize celery
+celery_app = init_celery(app)
+
+# Initialize scheduled tasks
+init_scheduled_tasks()
+
 
 # Server-side routes
 app.register_blueprint(tree_blueprint)
@@ -28,13 +33,15 @@
 @login_required
 def index():
     return render_template(
-        "index.html", is_dev=environ.get("FLASK_ENV") == "development"
+        "index.html",
+        is_dev=environ.get("FLASK_ENV") == "development",
     )
 
 
 @app.route("/app/<path:path>")
 @login_required
 def webpage(path):
     return render_template(
-        "index.html", is_dev=environ.get("FLASK_ENV") == "development"
+        "index.html",
+        is_dev=environ.get("FLASK_ENV") == "development",
     )

webapp/cache.py

@@ -10,19 +10,6 @@
 from redis import exceptions as redis_exceptions
 
 
-def init_cache(app: Flask):
-    try:
-        cache = RedisCache(app)
-    except Exception as e:
-        cache = FileCache(app)
-        app.logger.info(
-            f"Error: {e} Redis cache is not available."
-            " Using FileCache instead."
-        )
-    app.config["CACHE"] = cache
-    return cache
-
-
 class Cache(ABC):
     """Abstract Cache class"""
 
@@ -32,22 +19,18 @@ class Cache(ABC):
     @abstractmethod
     def get(self, key: str):
         """Get a value from the cache"""
-        pass
 
     @abstractmethod
     def set(self, key: str, value: Any):
         """Set a value in the cache"""
-        pass
 
     @abstractmethod
     def delete(self, key: str):
         """Delete a value from the cache"""
-        pass
 
     @abstractmethod
     def is_available(self):
         """Check if the cache is available"""
-        pass
 
 
 class RedisCache(Cache):
@@ -139,7 +122,8 @@ def connect(self):
 
         path_exists = Path(self.cache_path).exists()
         path_writable = Path(self.cache_path).is_dir() and os.access(
-            self.cache_path, os.W_OK
+            self.cache_path,
+            os.W_OK,
         )
         if not path_exists and path_writable:
             raise ConnectionError("Cache directory is not writable")
@@ -151,7 +135,13 @@ def save_to_file(self, key: str, value: Any):
         data = json.dumps(value)
         # Delete the file if it exists
         if Path(self.cache_path + "/" + key).exists():
-            os.remove(self.cache_path + "/" + key)
+            try:
+                os.remove(self.cache_path + "/" + key)
+            except FileNotFoundError:
+                # We catch this as due to different processes potentially
+                # accessing this method, deleting a file could face a race
+                # condition.
+                self.logger.info("File already removed")
         # Create base directory if it does not exist
         if not Path(self.cache_path).exists():
             Path(self.cache_path).mkdir(parents=True, exist_ok=True)
@@ -165,7 +155,7 @@ def load_from_file(self, key: str):
         # Check if the file exists
         if not Path(self.cache_path + "/" + key).exists():
             return None
-        with open(self.cache_path + "/" + key, "r") as f:
+        with open(self.cache_path + "/" + key) as f:
             data = f.read()
         return json.loads(data)
 
@@ -187,3 +177,15 @@ def onerror(*args, **kwargs):
             os.chmod(self.cache_path + "/" + key, 0o777)
 
         return shutil.rmtree(self.cache_path + "/" + key, onerror=onerror)
+
+
+def init_cache(app: Flask) -> Cache:
+    try:
+        cache = RedisCache(app)
+    except Exception as e:
+        cache = FileCache(app)
+        msg = f"Error: {e} Redis cache is not available."
+        " Using FileCache instead."
+        app.logger.info(msg)
+    app.config["CACHE"] = cache
+    return cache