Add new LocalFilesystemToS3Operator under Amazon provider (#17168)#17382
Merged
potiuk merged 7 commits intoapache:mainfrom Aug 14, 2021
Merged
Add new LocalFilesystemToS3Operator under Amazon provider (#17168)#17382potiuk merged 7 commits intoapache:mainfrom
potiuk merged 7 commits intoapache:mainfrom
Conversation
Fix `airflow celery stop` to accept the pid file. (apache#17278) Fix isort (apache#17330) Fix failing celery test (apache#17337) This change fixes failing test due to mocking
|
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/main/CONTRIBUTING.rst)
|
subkanthi
suggested changes
Aug 3, 2021
airflow/providers/amazon/aws/example_dags/example_local_to_s3.py
Outdated
Show resolved
Hide resolved
ferruzzi
reviewed
Aug 10, 2021
ferruzzi
reviewed
Aug 10, 2021
When dest_key is given as a full s3:// filepath and dest_bucket is also provided, operator throws an error in its init() method. A test is added for this case.
… given When dest_key is given as a full s3:// file path, the parent operator does not expect a dest_bucket. A TypeError is raised when dest_bucket is not None in this case.
mik-laj
reviewed
Aug 14, 2021
Member
mik-laj
left a comment
There was a problem hiding this comment.
LGTM, But CI hasn't been running for this change yet
potiuk
approved these changes
Aug 14, 2021
|
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease. |
|
Awesome work, congrats on your first merged pull request! |
45 tasks
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add new LocalFilesystemToS3Operator under Amazon provider (#17168)
Currently, an S3Hook exists that allows transfer of files to S3 via
load_file(), however there is no operator associated with it. The LocalFilesystemToS3Operator would wrap the S3 Hook, so it is not used directly.Seeing as to upload a local file to S3 using the S3 Hook, a Python task with the same functionality has to be written anyway, this could reduce a lot of redundant boiler-plate code and standardize the local file to S3 load process.
closes: #17168