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 22:16:27 UTC
[GitHub] [airflow] OmairK opened a new pull request #7726: [AIRFLOW-4175]
S3Hook load_file should support ACL policy parameter
OmairK opened a new pull request #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726
- Added acl_policy parameter to all the S3Hook.load_*() and S3Hook.copy_object() functions
- Added unittest to test the response permissions when the policy is passed
- Updated the docstring of the function
---
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>
- [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] OmairK edited a comment on issue #7726: [AIRFLOW-4175]
S3Hook load_file should support ACL policy parameter
Posted by GitBox <gi...@apache.org>.
OmairK edited a comment on issue #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726#issuecomment-599255128
> @OmairK -> I think you will have to rebase and push the branch again so that we can re-open it.
@potiuk I have rebased and forced pushed the branch.
----------------------------------------------------------------
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] feluelle commented on issue #7726: [AIRFLOW-4175] S3Hook
load_file should support ACL policy parameter
Posted by GitBox <gi...@apache.org>.
feluelle commented on issue #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726#issuecomment-599445514
Thank you @retornam and @OmairK for co-authoring this :) 👍
So https://github.com/apache/airflow/pull/7733 is the new one and we can close https://github.com/apache/airflow/pull/7635, right?
----------------------------------------------------------------
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] OmairK commented on issue #7726: [AIRFLOW-4175] S3Hook
load_file should support ACL policy parameter
Posted by GitBox <gi...@apache.org>.
OmairK commented on issue #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726#issuecomment-599215817
> It looks good. But one thing - can you also extend the tests to verify that the default "private" acl has also the right permissions ?
You are talking about the copy_object() hook ?
----------------------------------------------------------------
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] feluelle closed pull request #7726: [AIRFLOW-4175] S3Hook
load_file should support ACL policy parameter
Posted by GitBox <gi...@apache.org>.
feluelle closed pull request #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726
----------------------------------------------------------------
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] OmairK commented on issue #7726: [AIRFLOW-4175] S3Hook
load_file should support ACL policy parameter
Posted by GitBox <gi...@apache.org>.
OmairK commented on issue #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726#issuecomment-599255128
> @OmairK -> I think you will have to rebase and push the branch again so that we can re-open it.
@potiuk sure on 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] OmairK commented on issue #7726: [AIRFLOW-4175] S3Hook
load_file should support ACL policy parameter
Posted by GitBox <gi...@apache.org>.
OmairK commented on issue #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726#issuecomment-599265008
> And BTW - both @OmairK and @retornam can be co-authors of the change https://help.github.com/en/github/committing-changes-to-your-project/creating-a-commit-with-multiple-authors - we've done that multiple times and this is true open-source spirit ... - so why don't you add @retornam co-author line to the change :)?
Sure I will do that right now.
> @OmairK I can close mine, when you submit a new PR.
Thanks @retornam :smile:
----------------------------------------------------------------
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] retornam commented on issue #7726: [AIRFLOW-4175] S3Hook
load_file should support ACL policy parameter
Posted by GitBox <gi...@apache.org>.
retornam commented on issue #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726#issuecomment-599264509
@OmairK I can close mine, when you submit a new PR.
----------------------------------------------------------------
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 #7726: [AIRFLOW-4175] S3Hook
load_file should support ACL policy parameter
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726#issuecomment-599264312
And BTW - both @OmairK and @retornam can be co-authors of the change https://help.github.com/en/github/committing-changes-to-your-project/creating-a-commit-with-multiple-authors - we've done that multiple times and this is true open-source spirit ... - so why don't you add @retornam co-author line to the change :)?
----------------------------------------------------------------
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] feluelle commented on issue #7726: [AIRFLOW-4175] S3Hook
load_file should support ACL policy parameter
Posted by GitBox <gi...@apache.org>.
feluelle commented on issue #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726#issuecomment-599233883
I am sorry, but this is a duplicate. #7635 already adds ACL Header.
----------------------------------------------------------------
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] OmairK commented on issue #7726: [AIRFLOW-4175] S3Hook
load_file should support ACL policy parameter
Posted by GitBox <gi...@apache.org>.
OmairK commented on issue #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726#issuecomment-599247922
> I am sorry, but this is a duplicate. #7635 already adds ACL Header.
@feluelle I have already completed the task and made the changes @potiuk suggested, and since this issue was not explicitly assigned to anybody I have been working on this issue for the past 2 days.
----------------------------------------------------------------
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 #7726: [AIRFLOW-4175] S3Hook
load_file should support ACL policy parameter
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726#issuecomment-599254033
I think we can combine the two.. There are more tests in this one - so I think we should use it :). @retornam - how about you review this one instead of merging yours :)? Let's join forces.
----------------------------------------------------------------
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 #7726: [AIRFLOW-4175] S3Hook
load_file should support ACL policy parameter
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726#issuecomment-599264373
Somehow I can't re-open it :(. Likely you need to reopen it yourself @OmairK or create a new one with new branch name
----------------------------------------------------------------
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 #7726: [AIRFLOW-4175] S3Hook
load_file should support ACL policy parameter
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726#issuecomment-599254110
@OmairK -> I think you will have to rebase and push the branch again so that we can re-open 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] potiuk commented on issue #7726: [AIRFLOW-4175] S3Hook
load_file should support ACL policy parameter
Posted by GitBox <gi...@apache.org>.
potiuk commented on issue #7726: [AIRFLOW-4175] S3Hook load_file should support ACL policy parameter
URL: https://github.com/apache/airflow/pull/7726#issuecomment-599448496
Right @feluelle !
----------------------------------------------------------------
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