[GitHub] [airflow] mik-laj commented on a change in pull request #9030: Allow using Airflow with Flask CLI
mik-laj commented on a change in pull request #9030: URL: https://github.com/apache/airflow/pull/9030#discussion_r433541076 ## File path: tests/cli/commands/test_task_command.py ## @@ -170,9 +170,10 @@ def test_task_state(self): def test_task_states_for_dag_run(self): dag2 = DagBag().dags['example_python_operator'] - task2 = dag2.get_task(task_id='print_the_context') defaut_date2 = timezone.make_aware(datetime(2016, 1, 9)) +dag2.clear() Review comment: We may have data from another run. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #9030: Allow using Airflow with Flask CLI
mik-laj commented on a change in pull request #9030:
URL: https://github.com/apache/airflow/pull/9030#discussion_r433541242
##
File path: tests/cli/commands/test_task_command.py
##
@@ -201,7 +202,7 @@ def test_task_states_for_dag_run(self):
tablefmt="plain")
# Check that prints, and log messages, are shown
-self.assertEqual(expected.replace("\n", ""), actual_out.replace("\n",
""))
+self.assertIn(expected.replace("\n", ""), actual_out.replace("\n", ""))
Review comment:
On the console, we have action_logger logs too.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #9030: Allow using Airflow with Flask CLI
mik-laj commented on a change in pull request #9030:
URL: https://github.com/apache/airflow/pull/9030#discussion_r433540921
##
File path: tests/cli/commands/test_celery_command.py
##
@@ -29,9 +29,6 @@
from airflow.configuration import conf
from tests.test_utils.config import conf_vars
-mock.patch('airflow.utils.cli.action_logging', lambda x: x).start()
Review comment:
This caused a side effect. I don't know why it affected this change.
Probably some session is now saving properly.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #9030: Allow using Airflow with Flask CLI
mik-laj commented on a change in pull request #9030:
URL: https://github.com/apache/airflow/pull/9030#discussion_r431991963
##
File path: airflow/www/app.py
##
@@ -70,6 +73,31 @@ def create_app(config=None, testing=False,
app_name="Airflow"):
app.json_encoder = AirflowJsonEncoder
csrf.init_app(app)
+
+def apply_middlewares(flask_app: Flask):
+# Apply DispatcherMiddleware
+base_url = urlparse(conf.get('webserver', 'base_url'))[2]
+if not base_url or base_url == '/':
+base_url = ""
+if base_url:
+flask_app.wsgi_app = DispatcherMiddleware( # type: ignore
Review comment:
We don't use it in tests - test_views. This is still needed in
production for some users.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #9030: Allow using Airflow with Flask CLI
mik-laj commented on a change in pull request #9030:
URL: https://github.com/apache/airflow/pull/9030#discussion_r431166489
##
File path: airflow/www/app.py
##
@@ -70,6 +73,31 @@ def create_app(config=None, testing=False,
app_name="Airflow"):
app.json_encoder = AirflowJsonEncoder
csrf.init_app(app)
+
+def apply_middlewares(flask_app: Flask):
+# Apply DispatcherMiddleware
+base_url = urlparse(conf.get('webserver', 'base_url'))[2]
+if not base_url or base_url == '/':
+base_url = ""
+if base_url:
+flask_app.wsgi_app = DispatcherMiddleware( # type: ignore
Review comment:
This middleware is now optional. We use addresses in the form of
`blocked` which do not work properly with this middleware. This middleware
expects the addresses to be in the form `/blocked`. However, if we don't use
this middleware, it doesn't matter.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #9030: Allow using Airflow with Flask CLI
mik-laj commented on a change in pull request #9030:
URL: https://github.com/apache/airflow/pull/9030#discussion_r430800228
##
File path: airflow/www/app.py
##
@@ -70,6 +73,27 @@ def create_app(config=None, testing=False,
app_name="Airflow"):
app.json_encoder = AirflowJsonEncoder
csrf.init_app(app)
+
+def apply_middlewares(flask_app: Flask):
+# Apply DispatcherMiddleware
+base_url = urlparse(conf.get('webserver', 'base_url'))[2]
+if not base_url or base_url == '/':
+base_url = ""
+flask_app.wsgi_app = DispatcherMiddleware(root_app, {base_url:
flask_app.wsgi_app}) # type: ignore
Review comment:
```
The actual WSGI application. This is not implemented in
:meth:`__call__` so that middlewares can be applied without
losing a reference to the app object. Instead of doing this::
app = MyMiddleware(app)
It's a better idea to do this instead::
app.wsgi_app = MyMiddleware(app.wsgi_app)
Then you still have the original application object around and
can continue to call methods on it.
```
https://github.com/pallets/flask/blob/330a3e3ddba712def955e7a2ccee92a205dfb656/src/flask/app.py#L2323
##
File path: airflow/www/app.py
##
@@ -39,15 +39,18 @@
from airflow.utils.json import AirflowJsonEncoder
from airflow.www.static_config import configure_manifest_files
-app = None # type: Any
-appbuilder = None # type: Optional[AppBuilder]
+app: Optional[Flask] = None
csrf = CSRFProtect()
log = logging.getLogger(__name__)
+def root_app(env, resp):
+resp(b'404 Not Found', [('Content-Type', 'text/plain')])
+return [b'Apache Airflow is not at this location']
+
+
def create_app(config=None, testing=False, app_name="Airflow"):
-global app, appbuilder
Review comment:
This causes a side effect. We want the cache to be modified only by the
cached_app method.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
