You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/12/07 07:07:38 UTC

[GitHub] [airflow] uranusjr commented on a change in pull request #20096: Fix mypy issues in www/views.py

uranusjr commented on a change in pull request #20096:
URL: https://github.com/apache/airflow/pull/20096#discussion_r763690864



##########
File path: airflow/www/views.py
##########
@@ -543,7 +544,8 @@ class AirflowBaseView(BaseView):
     }
 
     if not conf.getboolean('core', 'unit_test_mode'):
-        extra_args['sqlite_warning'] = settings.Session.bind.dialect.name == 'sqlite'
+        session: sqlalchemy.orm.session.Session = settings.Session
+        extra_args['sqlite_warning'] = session.bind.dialect.name == 'sqlite'

Review comment:
       Although it is likely better to fix this in `settings` instead since I’d expect this very issue to pop up for other modules.
   
   I think changing `Session: Optional[SASession] = None` there to just `Session: SASession` would fix the issue.

##########
File path: airflow/www/views.py
##########
@@ -30,12 +30,13 @@
 from functools import wraps
 from json import JSONDecodeError
 from operator import itemgetter
-from typing import Any, Callable, List, Optional, Set, Tuple, Union
+from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Union
 from urllib.parse import parse_qsl, unquote, urlencode, urlparse
 
 import lazy_object_proxy
 import markupsafe
 import nvd3
+import sqlalchemy
 import sqlalchemy as sqla

Review comment:
       There’s already an alias imported so you can use `sqla` for where you’re using `sqlalchemy` and onmit the import addition.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org