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/03/29 17:44:14 UTC
[GitHub] [airflow] dimberman opened a new pull request #15070: Add serve_logs endpoint for scheduler when using LocalExecutor
dimberman opened a new pull request #15070:
URL: https://github.com/apache/airflow/pull/15070
Currently, the serve_logs endpoint only exists on Celery workers. This
means if someone launches Airflow with the LocalExecutor and wants to
grab the logs from the scheduler, there is no way to move that to the
webserver if it is on a different pod/machine
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
--
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:
users@infra.apache.org
[GitHub] [airflow] kaxil closed pull request #15070: Add serve_logs endpoint for scheduler when using LocalExecutor
Posted by GitBox <gi...@apache.org>.
kaxil closed pull request #15070:
URL: https://github.com/apache/airflow/pull/15070
--
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:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #15070: Add serve_logs endpoint for scheduler when using LocalExecutor
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15070:
URL: https://github.com/apache/airflow/pull/15070#discussion_r604073416
##########
File path: airflow/cli/commands/scheduler_command.py
##########
@@ -17,20 +17,27 @@
"""Scheduler command"""
import signal
+from multiprocessing import Process
+from typing import Optional
import daemon
from daemon.pidfile import TimeoutPIDLockFile
from airflow import settings
+from airflow.configuration import conf
from airflow.jobs.scheduler_job import SchedulerJob
from airflow.utils import cli as cli_utils
from airflow.utils.cli import process_subdir, setup_locations, setup_logging, sigint_handler, sigquit_handler
+from airflow.utils.serve_logs import serve_logs
@cli_utils.action_logging
def scheduler(args):
"""Starts Airflow Scheduler"""
print(settings.HEADER)
+ skip_serve_logs = args.skip_serve_logs
Review comment:
This won't currently be set for scheduler sub-command -- you'll need to add it somewhere in the cli parser.
--
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:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #15070: Add serve_logs endpoint for scheduler when using LocalExecutor
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15070:
URL: https://github.com/apache/airflow/pull/15070#discussion_r604074238
##########
File path: airflow/cli/commands/scheduler_command.py
##########
@@ -60,4 +69,17 @@ def scheduler(args):
signal.signal(signal.SIGINT, sigint_handler)
signal.signal(signal.SIGTERM, sigint_handler)
signal.signal(signal.SIGQUIT, sigquit_handler)
+ if is_local_executor:
+ sub_proc = _serve_logs(skip_serve_logs)
Review comment:
Can you check that when sending a SIGINT (i.e. ctrl-c) that the log server shuts down.
--
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:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #15070: Add serve_logs endpoint for scheduler when using LocalExecutor
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #15070:
URL: https://github.com/apache/airflow/pull/15070#discussion_r604076313
##########
File path: airflow/cli/commands/scheduler_command.py
##########
@@ -60,4 +69,17 @@ def scheduler(args):
signal.signal(signal.SIGINT, sigint_handler)
signal.signal(signal.SIGTERM, sigint_handler)
signal.signal(signal.SIGQUIT, sigquit_handler)
+ if is_local_executor:
+ sub_proc = _serve_logs(skip_serve_logs)
job.run()
+ if sub_proc:
+ sub_proc.terminate()
+
+
+def _serve_logs(skip_serve_logs: bool = False) -> Optional[Process]:
Review comment:
I know you coped this from the celery worker fn, but this is odd having the condition inside and passing `skip_server_logs` to it, rather than doing `if is_local_executor and not skip_serve_logs:` outside.
--
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:
users@infra.apache.org