Skip to content

[AIRFLOW-5834] Option to skip serve_logs process with workers#6485

Closed
msumit wants to merge 2 commits into
apache:masterfrom
msumit:AIRFLOW-5834
Closed

[AIRFLOW-5834] Option to skip serve_logs process with workers#6485
msumit wants to merge 2 commits into
apache:masterfrom
msumit:AIRFLOW-5834

Conversation

@msumit

@msumit msumit commented Nov 1, 2019

Copy link
Copy Markdown
Contributor

Make sure you have checked all steps below.

Jira

Description

  • Provide an option to skip serve_logs process along with workers

Tests

  • Added

Commits

  • My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does
    • If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

@feluelle

feluelle commented Nov 1, 2019

Copy link
Copy Markdown
Member

@msumit master is currently failing. A potential fix #6486 is already there. Therefore I cancelled your tests. Can you do a rebase after the PR is merged, please? :)

@mik-laj

mik-laj commented Nov 1, 2019

Copy link
Copy Markdown
Member

I am not sure if this is the best solution. We will add an additional switch, but the community will be low when we should use it. This server is only needed when using FileTaskHandler. Do you think it is possible to make this server start only when this logger is selected?

@msumit

msumit commented Nov 1, 2019

Copy link
Copy Markdown
Contributor Author

Thanks @feluelle.

@mik-laj agrees that it'll add another hook to the system. However, I'm not sure if starting a log server process for FileTasKHandler is correct, as I was using that only and writing logs to a persistent volume, and don't need this process at all.

IMO we should not start this process along with workers and let folks decide if they really need it or not?

@OmerJog

OmerJog commented Nov 3, 2019

Copy link
Copy Markdown
Contributor

@msumit can you take a look at the test you added?

test_serve_logs_on_worker_start (tests.cli.test_cli.TestCLI) ... WARNI [airflow.utils.log.logging_mixin.LoggingMixin] Starting serve logs process within worker is going to be deprecated, and will be removed from Airflow 2.0
Running a worker with superuser privileges when the
worker accepts messages serialized with pickle is a very bad idea!
If you really want to continue then you have to set the C_FORCE_ROOT
environment variable (but please think about this before you do).

@msumit

msumit commented Nov 3, 2019

Copy link
Copy Markdown
Contributor Author

@OmerJog as of now I'm not able to set that var C_FORCE_ROOT while invoking the worker command. Had asked the same in Slack as well, but got no response so far.

@msumit msumit closed this Dec 2, 2019
@msumit
msumit deleted the AIRFLOW-5834 branch December 2, 2019 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants