Proposal: Add ruff (linter and formatter)

.github/workflows/tests.yml

@@ -49,7 +49,7 @@ jobs:
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip
-          pip install -r requirements_test.txt
+          pip install -r dev-requirements.txt
       - name: Create shared_dir
         run: |
           mkdir shared_dir

.pre-commit-config.yaml

@@ -0,0 +1,10 @@
+exclude: ^static/.*|assets/.*|/migrations/.*|\.min\.js$|\.min\.css$|\.css\.map$|\.min\.js$|\.js\.map$|\.svg$
+default_language_version:
+  python: python3.10
+repos:
+  - repo: https://github.com/astral-sh/ruff-pre-commit
+    rev: v0.2.1
+    hooks:
+      - id: ruff
+        args: [ --fix ]
+      - id: ruff-format

Makefile

@@ -0,0 +1,19 @@
+export SUPERSET_CONFIG_PATH := superset_config.py
+export FLASK_APP := superset
+
+all: setup
+
+setup:
+	pip install -e .
+	pip install -r dev-requirements.txt
+	superset db upgrade
+	superset init
+
+load-examples:
+	superset load_examples
+
+create-admin:
+	superset fab create-admin
+
+runserver:
+	superset run -p 8088 --with-threads --reload --debugger

README.md

@@ -19,6 +19,12 @@ directly without another `pip install`.
   the config appropriately.
 - Run `pip install -e .`
 
+For formatting and linting, we use ruff. This can be installed as a pre-commit hook so it gets run on each commit. This tool will be installed when you install `dev-requirements.txt`
+- `pip install -r dev-requirements.txt`
+
+Now that the `pre-commit` tool is installed, we need to run the following to install the ruff config.
+- `pre-commit install`
+
 ### CommCare HQ OAuth Integration
 
 - Create an OAuth application on CommCare HQ using Django Admin
@@ -100,9 +106,9 @@ Here is how celery can be run locally.
 
 ### Testing
 
-Tests use pytest, which is included in `requirements_dev.txt`:
+Tests use pytest, which is included in `dev-requirements.txt`:
 
-    $ pip install -r requirements_test.txt
+    $ pip install -r dev-requirements.txt
     $ pytest
 
 The test runner can only run tests that do not import from Superset. The

requirements_test.txt → dev-requirements.txt

@@ -1,3 +1,5 @@
 -e file:.
 pytest==7.3.1
 Flask-Testing==0.8.1
+pre-commit==3.6.1
+ruff==0.2.1

hq_superset/__init__.py

@@ -8,10 +8,17 @@ def flask_app_mutator(app):
     # Import the views (which assumes the app is initialized) here
     # return
     from superset.extensions import appbuilder
+
     from . import api, hq_domain, views
 
-    appbuilder.add_view(views.HQDatasourceView, 'Update HQ Datasource', menu_cond=lambda *_: False)
-    appbuilder.add_view(views.SelectDomainView, 'Select a Domain', menu_cond=lambda *_: False)
+    appbuilder.add_view(
+        views.HQDatasourceView,
+        'Update HQ Datasource',
+        menu_cond=lambda *_: False,
+    )
+    appbuilder.add_view(
+        views.SelectDomainView, 'Select a Domain', menu_cond=lambda *_: False
+    )
     appbuilder.add_api(api.OAuth)
     appbuilder.add_api(api.DataSetChangeAPI)
     app.before_request_funcs.setdefault(None, []).append(
@@ -27,19 +34,24 @@ def flask_app_mutator(app):
 def override_jinja2_template_loader(app):
     # Allow loading templates from the templates directory in this project as well
 
-    template_path = os.sep.join((
-        os.path.dirname(os.path.abspath(__file__)),
-        'templates'
-    ))
-    my_loader = jinja2.ChoiceLoader([
+    template_path = os.sep.join(
+        (os.path.dirname(os.path.abspath(__file__)), 'templates')
+    )
+    my_loader = jinja2.ChoiceLoader(
+        [
             jinja2.FileSystemLoader([template_path]),
             app.jinja_loader,
-        ])
+        ]
+    )
     app.jinja_loader = my_loader
 
-    images_path = os.sep.join((
-        os.path.dirname(os.path.abspath(__file__)),
-        'images'
-    ))
-    blueprint = Blueprint('Static', __name__, static_url_path='/static/images', static_folder=images_path)
+    images_path = os.sep.join(
+        (os.path.dirname(os.path.abspath(__file__)), 'images')
+    )
+    blueprint = Blueprint(
+        'Static',
+        __name__,
+        static_url_path='/static/images',
+        static_folder=images_path,
+    )
     app.register_blueprint(blueprint)

hq_superset/api.py

@@ -46,7 +46,11 @@ def save_token(token, request):
 
 class HQBearerTokenValidator(BearerTokenValidator):
     def authenticate_token(self, token_string):
-        return db.session.query(Token).filter_by(access_token=token_string).first()
+        return (
+            db.session.query(Token)
+            .filter_by(access_token=token_string)
+            .first()
+        )
 
 
 require_oauth.register_token_validator(HQBearerTokenValidator())
@@ -60,22 +64,21 @@ def authenticate_token(self, token_string):
 
 
 class OAuth(BaseApi):
-
     def __init__(self):
         super().__init__()
-        self.route_base = "/oauth"
+        self.route_base = '/oauth'
 
-    @expose("/token", methods=('POST',))
+    @expose('/token', methods=('POST',))
     def issue_access_token(self):
         try:
             response = authorization.create_token_response()
         except NoResultFound:
-            return jsonify({"error": "Invalid client"}), 401
+            return jsonify({'error': 'Invalid client'}), 401
 
         if response.status_code >= 400:
             return response
 
-        data = json.loads(response.data.decode("utf-8"))
+        data = json.loads(response.data.decode('utf-8'))
         return jsonify(data)
 
 

hq_superset/hq_domain.py

@@ -10,40 +10,41 @@ def before_request_hook():
 def after_request_hook(response):
     # On logout clear domain cookie
     logout_views = [
-        "AuthDBView.login",
-        "AuthOAuthView.logout",
+        'AuthDBView.login',
+        'AuthOAuthView.logout',
     ]
-    if (request.url_rule and request.url_rule.endpoint in logout_views):
+    if request.url_rule and request.url_rule.endpoint in logout_views:
         response.set_cookie('hq_domain', '', expires=0)
     return response
 
 
 DOMAIN_EXCLUDED_VIEWS = [
-    "AuthOAuthView.login",
-    "AuthOAuthView.logout",
-    "AuthOAuthView.oauth_authorized",
-    "AuthDBView.logout",
-    "AuthDBView.login",
-    "SelectDomainView.list",
-    "SelectDomainView.select",
-    "appbuilder.static",
-    "static",
+    'AuthOAuthView.login',
+    'AuthOAuthView.logout',
+    'AuthOAuthView.oauth_authorized',
+    'AuthDBView.logout',
+    'AuthDBView.login',
+    'SelectDomainView.list',
+    'SelectDomainView.select',
+    'appbuilder.static',
+    'static',
 ]
 
 
 def is_user_admin():
     from superset import security_manager
-    return security_manager.is_admin()
 
+    return security_manager.is_admin()
 
 
 def ensure_domain_selected():
     # Check if a hq_domain cookie is set
     #   Ensure necessary roles, permissions and DB schemas are created for the domain
-    if is_user_admin() or (request.url_rule and request.url_rule.endpoint in DOMAIN_EXCLUDED_VIEWS):
+    if is_user_admin() or (
+        request.url_rule and request.url_rule.endpoint in DOMAIN_EXCLUDED_VIEWS
+    ):
         return
     hq_domain = request.cookies.get('hq_domain')
-    valid_domains = user_domains()
     if is_valid_user_domain(hq_domain):
         g.hq_domain = hq_domain
     else:
@@ -60,14 +61,15 @@ def user_domains():
     # This should be set by oauth_user_info after OAuth
     if is_user_admin() or SESSION_USER_DOMAINS_KEY not in session:
         return []
-    return [
-        d["domain_name"]
-        for d in session[SESSION_USER_DOMAINS_KEY]
-    ]
+    return [d['domain_name'] for d in session[SESSION_USER_DOMAINS_KEY]]
 
 
 def add_domain_links(active_domain, domains):
     import superset
-    for domain in domains:
-        superset.appbuilder.menu.add_link(domain, category=active_domain, href=url_for('SelectDomainView.select', hq_domain=domain))
 
+    for domain in domains:
+        superset.appbuilder.menu.add_link(
+            domain,
+            category=active_domain,
+            href=url_for('SelectDomainView.select', hq_domain=domain),
+        )

hq_superset/hq_requests.py

@@ -1,27 +1,27 @@
 import superset
+
 from hq_superset.oauth import get_valid_cchq_oauth_token
 
 
 class HqUrl:
     @classmethod
     def datasource_export_url(cls, domain, datasource_id):
-        return f"a/{domain}/configurable_reports/data_sources/export/{datasource_id}/?format=csv"
+        return f'a/{domain}/configurable_reports/data_sources/export/{datasource_id}/?format=csv'
 
     @classmethod
     def datasource_list_url(cls, domain):
-        return f"a/{domain}/api/v0.5/ucr_data_source/"
+        return f'a/{domain}/api/v0.5/ucr_data_source/'
 
     @classmethod
     def datasource_details_url(cls, domain, datasource_id):
-        return f"a/{domain}/api/v0.5/ucr_data_source/{datasource_id}/"
+        return f'a/{domain}/api/v0.5/ucr_data_source/{datasource_id}/'
 
     @classmethod
     def subscribe_to_datasource_url(cls, domain, datasource_id):
-        return f"a/{domain}/configurable_reports/data_sources/subscribe/{datasource_id}/"
+        return f'a/{domain}/configurable_reports/data_sources/subscribe/{datasource_id}/'
 
 
 class HQRequest:
-
     def __init__(self, url):
         self.url = url
 
@@ -31,18 +31,20 @@ def oauth_token(self):
 
     @property
     def commcare_provider(self):
-        return superset.appbuilder.sm.oauth_remotes["commcare"]
+        return superset.appbuilder.sm.oauth_remotes['commcare']
 
     @property
     def api_base_url(self):
         return self.commcare_provider.api_base_url
 
     @property
     def absolute_url(self):
-        return f"{self.api_base_url}{self.url}"
+        return f'{self.api_base_url}{self.url}'
 
     def get(self):
         return self.commcare_provider.get(self.url, token=self.oauth_token)
 
     def post(self, data):
-        return self.commcare_provider.post(self.url, data=data, token=self.oauth_token)
+        return self.commcare_provider.post(
+            self.url, data=data, token=self.oauth_token
+        )

hq_superset/models.py

@@ -1,12 +1,12 @@
-import uuid
-import string
 import secrets
+import string
+import uuid
 from dataclasses import dataclass
+from datetime import datetime
 from typing import Any, Dict, Literal
 
-from datetime import datetime
-from superset import db
 from authlib.integrations.sqla_oauth2 import OAuth2ClientMixin
+from superset import db
 from werkzeug.security import check_password_hash, generate_password_hash
 
 
@@ -33,7 +33,7 @@ def revoke_tokens(self):
         tokens = db.session.execute(
             db.select(Token).filter_by(client_id=self.client_id, revoked=False)
         ).all()
-        for token, in tokens:
+        for (token,) in tokens:
             token.revoked = True
             db.session.add(token)
         db.session.commit()
@@ -44,7 +44,9 @@ def get_by_domain(cls, domain):
 
     @classmethod
     def get_by_client_id(cls, client_id):
-        return db.session.query(HQClient).filter_by(client_id=client_id).first()
+        return (
+            db.session.query(HQClient).filter_by(client_id=client_id).first()
+        )
 
     @classmethod
     def create_domain_client(cls, domain: str):
@@ -56,7 +58,7 @@ def create_domain_client(cls, domain: str):
             client_id=str(uuid.uuid4()),
             client_secret=generate_password_hash(client_secret),
         )
-        client.set_client_metadata({"grant_types": ["client_credentials"]})
+        client.set_client_metadata({'grant_types': ['client_credentials']})
 
         db.session.add(client)
         db.session.commit()