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 2022/08/11 10:26:35 UTC
[GitHub] [airflow] ashb opened a new pull request, #25664: Configurable umask to all deamonized processes.
ashb opened a new pull request, #25664:
URL: https://github.com/apache/airflow/pull/25664
We previously had this for just the `celery worker` subcommand, this PR
extends it to anything that can run in daemon mode.
The old celery.worker_umask is still respected, but not shown in the config anymore.
--
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
[GitHub] [airflow] ashb commented on a diff in pull request #25664: Configurable umask to all deamonized processes.
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #25664:
URL: https://github.com/apache/airflow/pull/25664#discussion_r943350690
##########
airflow/cli/cli_parser.py:
##########
@@ -671,7 +671,6 @@ def string_lower_type(val):
ARG_UMASK = Arg(
("-u", "--umask"),
help="Set the umask of celery worker in daemon mode",
Review Comment:
Not currently -- it's only used for the `celery worker` command -- the other commands don't have a `--umask` argument, they only use the config value.
--
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
[GitHub] [airflow] jedcunningham commented on a diff in pull request #25664: Configurable umask to all deamonized processes.
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #25664:
URL: https://github.com/apache/airflow/pull/25664#discussion_r943655559
##########
airflow/settings.py:
##########
@@ -640,3 +640,5 @@ def initialize():
# Prefix used to identify tables holding data moved during migration.
AIRFLOW_MOVED_TABLE_PREFIX = "_airflow_moved"
+
+DAEMON_UMASK: str = conf.get('core', 'daemon_umask', fallback='0o077')
Review Comment:
I see. One day I'll start my crusade against these fallbacks littered everywhere, but today is not that day.
--
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
[GitHub] [airflow] norm commented on a diff in pull request #25664: Configurable umask to all deamonized processes.
Posted by GitBox <gi...@apache.org>.
norm commented on code in PR #25664:
URL: https://github.com/apache/airflow/pull/25664#discussion_r943344487
##########
airflow/cli/cli_parser.py:
##########
@@ -671,7 +671,6 @@ def string_lower_type(val):
ARG_UMASK = Arg(
("-u", "--umask"),
help="Set the umask of celery worker in daemon mode",
Review Comment:
Help needs updating?
--
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
[GitHub] [airflow] norm commented on a diff in pull request #25664: Configurable umask to all deamonized processes.
Posted by GitBox <gi...@apache.org>.
norm commented on code in PR #25664:
URL: https://github.com/apache/airflow/pull/25664#discussion_r943345710
##########
airflow/config_templates/config.yml:
##########
@@ -374,6 +374,19 @@
example: ~
default: "1024"
+ - name: daemon_umask
+ description: |
+ The default umask to use for process when run in daemon mode (scheduler, worker, etc.)
+
+ This control the file-creation mode mask which determines the initial value of file permission bits
Review Comment:
*controls*
--
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
[GitHub] [airflow] potiuk commented on a diff in pull request #25664: Configurable umask to all deamonized processes.
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #25664:
URL: https://github.com/apache/airflow/pull/25664#discussion_r944336299
##########
airflow/settings.py:
##########
@@ -640,3 +640,5 @@ def initialize():
# Prefix used to identify tables holding data moved during migration.
AIRFLOW_MOVED_TABLE_PREFIX = "_airflow_moved"
+
+DAEMON_UMASK: str = conf.get('core', 'daemon_umask', fallback='0o077')
Review Comment:
"shields shall be splintered" some day ...
--
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
[GitHub] [airflow] ashb commented on a diff in pull request #25664: Configurable umask to all deamonized processes.
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #25664:
URL: https://github.com/apache/airflow/pull/25664#discussion_r943651995
##########
airflow/settings.py:
##########
@@ -640,3 +640,5 @@ def initialize():
# Prefix used to identify tables holding data moved during migration.
AIRFLOW_MOVED_TABLE_PREFIX = "_airflow_moved"
+
+DAEMON_UMASK: str = conf.get('core', 'daemon_umask', fallback='0o077')
Review Comment:
Oh this was sort of just to make typing happy -- otherwise this was `str | None`, but with the overload I added providing a fallback means it can't be None.
--
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
[GitHub] [airflow] ashb commented on a diff in pull request #25664: Configurable umask to all deamonized processes.
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #25664:
URL: https://github.com/apache/airflow/pull/25664#discussion_r943660247
##########
airflow/settings.py:
##########
@@ -640,3 +640,5 @@ def initialize():
# Prefix used to identify tables holding data moved during migration.
AIRFLOW_MOVED_TABLE_PREFIX = "_airflow_moved"
+
+DAEMON_UMASK: str = conf.get('core', 'daemon_umask', fallback='0o077')
Review Comment:
My crusade is to remove writing defaults to `airflow.cfg` (but still have the defaults config live in code)
--
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
[GitHub] [airflow] ashb merged pull request #25664: Configurable umask to all deamonized processes.
Posted by GitBox <gi...@apache.org>.
ashb merged PR #25664:
URL: https://github.com/apache/airflow/pull/25664
--
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
[GitHub] [airflow] jedcunningham commented on a diff in pull request #25664: Configurable umask to all deamonized processes.
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #25664:
URL: https://github.com/apache/airflow/pull/25664#discussion_r943619518
##########
airflow/settings.py:
##########
@@ -640,3 +640,5 @@ def initialize():
# Prefix used to identify tables holding data moved during migration.
AIRFLOW_MOVED_TABLE_PREFIX = "_airflow_moved"
+
+DAEMON_UMASK: str = conf.get('core', 'daemon_umask', fallback='0o077')
Review Comment:
```suggestion
DAEMON_UMASK: str = conf.get('core', 'daemon_umask')
```
nit: don't need this fallback.
--
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
[GitHub] [airflow] ashb commented on a diff in pull request #25664: Configurable umask to all deamonized processes.
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #25664:
URL: https://github.com/apache/airflow/pull/25664#discussion_r943351110
##########
airflow/config_templates/config.yml:
##########
@@ -374,6 +374,19 @@
example: ~
default: "1024"
+ - name: daemon_umask
+ description: |
+ The default umask to use for process when run in daemon mode (scheduler, worker, etc.)
+
+ This control the file-creation mode mask which determines the initial value of file permission bits
Review Comment:
```suggestion
This controls the file-creation mode mask which determines the initial value of file permission bits
```
##########
airflow/config_templates/default_airflow.cfg:
##########
@@ -212,6 +212,14 @@ default_pool_task_slot_count = 128
# mapped tasks from clogging the scheduler.
max_map_length = 1024
+# The default umask to use for process when run in daemon mode (scheduler, worker, etc.)
+#
+# This control the file-creation mode mask which determines the initial value of file permission bits
Review Comment:
```suggestion
# This controls the file-creation mode mask which determines the initial value of file permission bits
```
--
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
[GitHub] [airflow] risicle commented on pull request #25664: Configurable umask to all deamonized processes.
Posted by GitBox <gi...@apache.org>.
risicle commented on PR #25664:
URL: https://github.com/apache/airflow/pull/25664#issuecomment-1237454212
CVE-2022-38170 ?
--
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