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 2020/03/14 14:39:57 UTC
[GitHub] [airflow] aggarwaldeepak opened a new pull request #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
aggarwaldeepak opened a new pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724
When celery workers are run in daemon mode, umask was set to default(0)
which create dirs/files with 0777/0666 permissions.
---
Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [x] Commit message/PR title starts with `[AIRFLOW-NNNN]`. AIRFLOW-NNNN = JIRA ID<sup>*</sup>
- [ ] Unit tests coverage for changes (not needed for documentation changes)
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [ ] Relevant documentation is updated including usage instructions.
- [ ] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
<sup>*</sup> For document-only changes commit message can start with `[AIRFLOW-XXXX]`.
---
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).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] ivorynoise commented on issue #7724: [AIRFLOW-1536]
Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
ivorynoise commented on issue #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#issuecomment-615201304
@mik-laj I never meant to close this. I am still struggling to understand the git workflow so it just happened. I ll surely update this one. Please allow me today's time.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #7724: [AIRFLOW-1536] Inherit
umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#issuecomment-613703252
@potiuk @ivorynoise What is the progress of this PR? I see a rich discussion and acceptance of the change, but can I merge these changes? Is this change waiting for something?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] ivorynoise commented on a change in pull request #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
ivorynoise commented on a change in pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#discussion_r396081333
##########
File path: airflow/cli/commands/celery_command.py
##########
@@ -135,6 +135,7 @@ def worker(args):
ctx = daemon.DaemonContext(
files_preserve=[handle],
+ umask=cli_utils.get_umask(),
Review comment:
Thanks for explanation. Things are much clearer to me now!
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#discussion_r392692256
##########
File path: airflow/cli/commands/celery_command.py
##########
@@ -135,6 +135,7 @@ def worker(args):
ctx = daemon.DaemonContext(
files_preserve=[handle],
+ umask=cli_utils.get_umask(),
Review comment:
The problem is that you do not know what the umask is in the parent process - it depends on the system umask setting. It's a nuance but I think it's a good practice to have it fixed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io commented on issue #7724: [AIRFLOW-1536]
Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#issuecomment-599077938
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=h1) Report
> Merging [#7724](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/35eff0a15bedbbda74eec833ec8538beeaeaf777&el=desc) will **decrease** coverage by `0.92%`.
> The diff coverage is `25.00%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7724/graphs/tree.svg?width=650&height=150&src=pr&token=WdLKlKHOAU)](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7724 +/- ##
==========================================
- Coverage 86.92% 86.00% -0.93%
==========================================
Files 915 915
Lines 44152 44156 +4
==========================================
- Hits 38381 37975 -406
- Misses 5771 6181 +410
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/cli/commands/celery\_command.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvY2VsZXJ5X2NvbW1hbmQucHk=) | `64.00% <ø> (ø)` | |
| [airflow/utils/cli.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9jbGkucHk=) | `70.08% <25.00%> (-1.60%)` | :arrow_down: |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0.00%> (-72.16%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/api/auth/backend/kerberos\_auth.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9hcGkvYXV0aC9iYWNrZW5kL2tlcmJlcm9zX2F1dGgucHk=) | `28.16% <0.00%> (-54.93%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=footer). Last update [35eff0a...d0560e9](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#issuecomment-599077938
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=h1) Report
> Merging [#7724](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/35eff0a15bedbbda74eec833ec8538beeaeaf777?src=pr&el=desc) will **decrease** coverage by `0.77%`.
> The diff coverage is `25%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7724/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7724 +/- ##
==========================================
- Coverage 86.92% 86.15% -0.78%
==========================================
Files 915 915
Lines 44152 44156 +4
==========================================
- Hits 38381 38042 -339
- Misses 5771 6114 +343
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/cli/commands/celery\_command.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvY2VsZXJ5X2NvbW1hbmQucHk=) | `64% <ø> (ø)` | :arrow_up: |
| [airflow/utils/cli.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9jbGkucHk=) | `70.08% <25%> (-1.6%)` | :arrow_down: |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0%> (-72.16%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0%> (-64.16%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0%> (-55.56%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50% <0%> (-50%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50% <0%> (-50%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0%> (-47.06%)` | :arrow_down: |
| [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0%> (-46.67%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0%> (-45.08%)` | :arrow_down: |
| ... and [11 more](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=footer). Last update [35eff0a...d0560e9](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#issuecomment-599077938
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=h1) Report
> Merging [#7724](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/35eff0a15bedbbda74eec833ec8538beeaeaf777&el=desc) will **decrease** coverage by `27.08%`.
> The diff coverage is `89.58%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7724/graphs/tree.svg?width=650&height=150&src=pr&token=WdLKlKHOAU)](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7724 +/- ##
===========================================
- Coverage 86.92% 59.84% -27.09%
===========================================
Files 915 915
Lines 44152 44176 +24
===========================================
- Hits 38381 26436 -11945
- Misses 5771 17740 +11969
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/cli/commands/celery\_command.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvY2VsZXJ5X2NvbW1hbmQucHk=) | `64.00% <ø> (ø)` | |
| [...w/example\_dags/example\_latest\_only\_with\_trigger.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9sYXRlc3Rfb25seV93aXRoX3RyaWdnZXIucHk=) | `100.00% <ø> (ø)` | |
| [airflow/example\_dags/example\_subdag\_operator.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9zdWJkYWdfb3BlcmF0b3IucHk=) | `100.00% <ø> (ø)` | |
| [airflow/example\_dags/subdags/subdag.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3Mvc3ViZGFncy9zdWJkYWcucHk=) | `100.00% <ø> (ø)` | |
| [airflow/utils/cli.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9jbGkucHk=) | `70.08% <25.00%> (-1.60%)` | :arrow_down: |
| [...roviders/google/cloud/operators/postgres\_to\_gcs.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvZ29vZ2xlL2Nsb3VkL29wZXJhdG9ycy9wb3N0Z3Jlc190b19nY3MucHk=) | `54.28% <50.00%> (-31.01%)` | :arrow_down: |
| [airflow/models/base.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvYmFzZS5weQ==) | `93.75% <83.33%> (-6.25%)` | :arrow_down: |
| [airflow/models/dag.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvZGFnLnB5) | `91.21% <100.00%> (ø)` | |
| [airflow/models/log.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvbG9nLnB5) | `100.00% <100.00%> (ø)` | |
| [airflow/models/slamiss.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvc2xhbWlzcy5weQ==) | `93.33% <100.00%> (ø)` | |
| ... and [319 more](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=footer). Last update [35eff0a...ac5ba74](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] ivorynoise opened a new pull request #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
ivorynoise opened a new pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724
When celery workers are run in daemon mode, umask was set to default(0)
which create dirs/files with 0777/0666 permissions.
---
Issue link: [AIRFLOW-1536](https://issues.apache.org/jira/browse/AIRFLOW-1536)
Make sure to mark the boxes below before creating PR: [x]
- [x] Description above provides context of the change
- [x] Commit message/PR title starts with `[AIRFLOW-NNNN]`. AIRFLOW-NNNN = JIRA ID<sup>*</sup>
- [x] Unit tests coverage for changes (not needed for documentation changes)
- [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
- [x] Relevant documentation is updated including usage instructions.
- [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
<sup>*</sup> For document-only changes commit message can start with `[AIRFLOW-XXXX]`.
---
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).
Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] aggarwaldeepak commented on a change in pull request
#7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
aggarwaldeepak commented on a change in pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#discussion_r392688027
##########
File path: airflow/cli/commands/celery_command.py
##########
@@ -135,6 +135,7 @@ def worker(args):
ctx = daemon.DaemonContext(
files_preserve=[handle],
+ umask=cli_utils.get_umask(),
Review comment:
I have referred few articles to get into the depth but could find why the value should not depend on parent process or what repercussions can it have if I inherit it from parent process . However all articles suggest it to set to a predefined value. I will make the value configurable with a reasonable default.
- https://smnd.sk/anino/programming/c/c-faq_comp.unix.programmer/faq_2.html
- http://www.enderunix.org/docs/eng/daemon.php
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#discussion_r392692428
##########
File path: airflow/cli/commands/celery_command.py
##########
@@ -135,6 +135,7 @@ def worker(args):
ctx = daemon.DaemonContext(
files_preserve=[handle],
+ umask=cli_utils.get_umask(),
Review comment:
Just to give some example - there are problems with Dockerfile caching when git by default uses umask of the systems (for example debian/mint had it different and different versions) rather than have predefined value. Different systems at different times had different default umask and that means that your daemon might behave differently for those different systems,
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] ivorynoise commented on a change in pull request #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
ivorynoise commented on a change in pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#discussion_r396118226
##########
File path: airflow/utils/cli.py
##########
@@ -238,3 +238,12 @@ def sigquit_handler(sig, frame): # pylint: disable=unused-argument
if line:
code.append(" {}".format(line.strip()))
print("\n".join(code))
+
+
+def get_umask():
+ """
+ Returns umask to control default file permission for new files
+ """
+ cur_mask = os.umask(0)
+ os.umask(cur_mask)
Review comment:
I have removed this part. Now umask isn't read from the parent.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] ivorynoise commented on issue #7724: [AIRFLOW-1536]
Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
ivorynoise commented on issue #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#issuecomment-615068460
Yeah sure. I will do it today. Was quite occupied with release at my
company.
On Wed, 15 Apr 2020 at 14:54, Jarek Potiuk <no...@github.com> wrote:
> @ivorynoise <https://github.com/ivorynoise> -> my bad. I totally missed
> that one. Can you please reabase and I will be happy to merge it!
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/airflow/pull/7724#issuecomment-613924955>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACVSPGTB27EKTWFAHSPZMOLRMV4LFANCNFSM4LJF37TQ>
> .
>
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] mik-laj commented on issue #7724: [AIRFLOW-1536] Inherit
umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
mik-laj commented on issue #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#issuecomment-615129048
Hi. This change seems to be significant, so closing this change hurts my heart. Can I help you?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] aggarwaldeepak commented on a change in pull request
#7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
aggarwaldeepak commented on a change in pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#discussion_r392611916
##########
File path: airflow/utils/cli.py
##########
@@ -238,3 +238,12 @@ def sigquit_handler(sig, frame): # pylint: disable=unused-argument
if line:
code.append(" {}".format(line.strip()))
print("\n".join(code))
+
+
+def get_umask():
+ """
+ Returns umask to control default file permission for new files
+ """
+ cur_mask = os.umask(0)
+ os.umask(cur_mask)
Review comment:
If I don't set it then the umask of for the rest of the execution is set to 0 and any files created by parent will be created with permissions 0666.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#issuecomment-599077938
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=h1) Report
> Merging [#7724](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/35eff0a15bedbbda74eec833ec8538beeaeaf777&el=desc) will **decrease** coverage by `0.77%`.
> The diff coverage is `25.00%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7724/graphs/tree.svg?width=650&height=150&src=pr&token=WdLKlKHOAU)](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7724 +/- ##
==========================================
- Coverage 86.92% 86.15% -0.78%
==========================================
Files 915 915
Lines 44152 44156 +4
==========================================
- Hits 38381 38042 -339
- Misses 5771 6114 +343
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/cli/commands/celery\_command.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvY2VsZXJ5X2NvbW1hbmQucHk=) | `64.00% <ø> (ø)` | |
| [airflow/utils/cli.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy91dGlscy9jbGkucHk=) | `70.08% <25.00%> (-1.60%)` | :arrow_down: |
| [...flow/providers/apache/cassandra/hooks/cassandra.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2Nhc3NhbmRyYS9ob29rcy9jYXNzYW5kcmEucHk=) | `21.51% <0.00%> (-72.16%)` | :arrow_down: |
| [...w/providers/apache/hive/operators/mysql\_to\_hive.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvYXBhY2hlL2hpdmUvb3BlcmF0b3JzL215c3FsX3RvX2hpdmUucHk=) | `35.84% <0.00%> (-64.16%)` | :arrow_down: |
| [airflow/kubernetes/volume\_mount.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZV9tb3VudC5weQ==) | `44.44% <0.00%> (-55.56%)` | :arrow_down: |
| [airflow/providers/postgres/operators/postgres.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcG9zdGdyZXMvb3BlcmF0b3JzL3Bvc3RncmVzLnB5) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/providers/redis/operators/redis\_publish.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvcmVkaXMvb3BlcmF0b3JzL3JlZGlzX3B1Ymxpc2gucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [airflow/kubernetes/volume.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3ZvbHVtZS5weQ==) | `52.94% <0.00%> (-47.06%)` | :arrow_down: |
| [airflow/providers/mongo/sensors/mongo.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9wcm92aWRlcnMvbW9uZ28vc2Vuc29ycy9tb25nby5weQ==) | `53.33% <0.00%> (-46.67%)` | :arrow_down: |
| [airflow/kubernetes/pod\_launcher.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9rdWJlcm5ldGVzL3BvZF9sYXVuY2hlci5weQ==) | `47.18% <0.00%> (-45.08%)` | :arrow_down: |
| ... and [11 more](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=footer). Last update [35eff0a...d0560e9](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on issue #7724: [AIRFLOW-1536] Inherit
umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#issuecomment-613924955
@ivorynoise -> my bad. I totally missed that one. Can you please reabase and I will be happy to merge it!
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] boring-cyborg[bot] commented on issue #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#issuecomment-616195563
Awesome work, congrats on your first merged pull request!
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#discussion_r392692428
##########
File path: airflow/cli/commands/celery_command.py
##########
@@ -135,6 +135,7 @@ def worker(args):
ctx = daemon.DaemonContext(
files_preserve=[handle],
+ umask=cli_utils.get_umask(),
Review comment:
Just to give some example - there are problems with Dockerfile caching when git by default uses umask of the system rather than have predefined value. Different systems at different times had different default umask (for example debian/mint had it different and different versions) and that means that your daemon might behave differently for those different systems,
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#discussion_r392602337
##########
File path: airflow/cli/commands/celery_command.py
##########
@@ -135,6 +135,7 @@ def worker(args):
ctx = daemon.DaemonContext(
files_preserve=[handle],
+ umask=cli_utils.get_umask(),
Review comment:
I think rather than reading from a parent we should allow to set it to a fixed value from configuration (with a reasonable default - for example 0o077).
From the documentation of umask parameter in DaemonContext:
```
File access creation mask (“umask”) to set for the process on daemon start.
A daemon should not rely on the parent process's umask value, which is beyond its control and may
prevent creating a file with the required access mode. So when the daemon context opens, the umask
is set to an explicit known value.
If the conventional value of 0 is too open, consider setting a value such as 0o022, 0o027, 0o077, or
another specific value. Otherwise, ensure the daemon creates every file with an explicit access mode
for the purpose.
```
via https://kite.com/python/docs/daemon.DaemonContext
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#discussion_r392612429
##########
File path: airflow/utils/cli.py
##########
@@ -238,3 +238,12 @@ def sigquit_handler(sig, frame): # pylint: disable=unused-argument
if line:
code.append(" {}".format(line.strip()))
print("\n".join(code))
+
+
+def get_umask():
+ """
+ Returns umask to control default file permission for new files
+ """
+ cur_mask = os.umask(0)
+ os.umask(cur_mask)
Review comment:
I see - then it's yet another reason why we should not set/reset the umask. We really do not need to find out what is the current umask - we should set a reasonable value from configuration.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] codecov-io edited a comment on issue #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#issuecomment-599077938
# [Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=h1) Report
> Merging [#7724](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=desc) into [master](https://codecov.io/gh/apache/airflow/commit/35eff0a15bedbbda74eec833ec8538beeaeaf777?src=pr&el=desc) will **decrease** coverage by `0.31%`.
> The diff coverage is `91.66%`.
[![Impacted file tree graph](https://codecov.io/gh/apache/airflow/pull/7724/graphs/tree.svg?width=650&token=WdLKlKHOAU&height=150&src=pr)](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #7724 +/- ##
==========================================
- Coverage 86.92% 86.61% -0.32%
==========================================
Files 915 915
Lines 44152 44176 +24
==========================================
- Hits 38381 38263 -118
- Misses 5771 5913 +142
```
| [Impacted Files](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [airflow/example\_dags/subdags/subdag.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3Mvc3ViZGFncy9zdWJkYWcucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [...w/example\_dags/example\_latest\_only\_with\_trigger.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9sYXRlc3Rfb25seV93aXRoX3RyaWdnZXIucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [airflow/example\_dags/example\_subdag\_operator.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9leGFtcGxlX2RhZ3MvZXhhbXBsZV9zdWJkYWdfb3BlcmF0b3IucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [airflow/cli/commands/celery\_command.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9jbGkvY29tbWFuZHMvY2VsZXJ5X2NvbW1hbmQucHk=) | `64% <ø> (ø)` | :arrow_up: |
| [airflow/models/taskfail.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2ZhaWwucHk=) | `100% <100%> (ø)` | :arrow_up: |
| [airflow/models/slamiss.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvc2xhbWlzcy5weQ==) | `93.33% <100%> (ø)` | :arrow_up: |
| [airflow/models/log.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvbG9nLnB5) | `100% <100%> (ø)` | :arrow_up: |
| [airflow/models/taskreschedule.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza3Jlc2NoZWR1bGUucHk=) | `100% <100%> (ø)` | :arrow_up: |
| [airflow/stats.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9zdGF0cy5weQ==) | `86.07% <100%> (+2.51%)` | :arrow_up: |
| [airflow/models/taskinstance.py](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree#diff-YWlyZmxvdy9tb2RlbHMvdGFza2luc3RhbmNlLnB5) | `94.97% <100%> (ø)` | :arrow_up: |
| ... and [13 more](https://codecov.io/gh/apache/airflow/pull/7724/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=footer). Last update [35eff0a...ac5ba74](https://codecov.io/gh/apache/airflow/pull/7724?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] aggarwaldeepak commented on a change in pull request
#7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
aggarwaldeepak commented on a change in pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#discussion_r392611916
##########
File path: airflow/utils/cli.py
##########
@@ -238,3 +238,12 @@ def sigquit_handler(sig, frame): # pylint: disable=unused-argument
if line:
code.append(" {}".format(line.strip()))
print("\n".join(code))
+
+
+def get_umask():
+ """
+ Returns umask to control default file permission for new files
+ """
+ cur_mask = os.umask(0)
+ os.umask(cur_mask)
Review comment:
If I don't set it then the umask for the rest of the execution is set to 0 and any files created by parent will be created with permissions 0666.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] ivorynoise commented on a change in pull request #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
ivorynoise commented on a change in pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#discussion_r396118370
##########
File path: airflow/cli/commands/celery_command.py
##########
@@ -135,6 +135,7 @@ def worker(args):
ctx = daemon.DaemonContext(
files_preserve=[handle],
+ umask=cli_utils.get_umask(),
Review comment:
I have incorporated the changes as suggested. Umask is read from configuration. Also updated corresponding docs and configuration file.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] ivorynoise closed pull request #7724: [AIRFLOW-1536]
Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
ivorynoise closed pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] boring-cyborg[bot] commented on issue #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on issue #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#issuecomment-599071486
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, pylint and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/master/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/master/docs/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/master/BREEZE.rst) for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://apache-airflow-slack.herokuapp.com/
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk commented on a change in pull request #7724:
[AIRFLOW-1536] Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#discussion_r392602040
##########
File path: airflow/utils/cli.py
##########
@@ -238,3 +238,12 @@ def sigquit_handler(sig, frame): # pylint: disable=unused-argument
if line:
code.append(" {}".format(line.strip()))
print("\n".join(code))
+
+
+def get_umask():
+ """
+ Returns umask to control default file permission for new files
+ """
+ cur_mask = os.umask(0)
+ os.umask(cur_mask)
Review comment:
Why do you set it here ? I think this is not necessary.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] potiuk merged pull request #7724: [AIRFLOW-1536] Inherit
umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [airflow] ivorynoise commented on issue #7724: [AIRFLOW-1536]
Inherit umask from parent process in daemon mode
Posted by GitBox <gi...@apache.org>.
ivorynoise commented on issue #7724: [AIRFLOW-1536] Inherit umask from parent process in daemon mode
URL: https://github.com/apache/airflow/pull/7724#issuecomment-615955122
@mik-laj @potiuk request you to review the changes and merge with the master. Thanks.
----------------------------------------------------------------
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
With regards,
Apache Git Services