-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Add oss_task_handler into alibaba-provider and enable remote logging to OSS #21785
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
EricGao888
commented
Feb 24, 2022
- In order to add more support for Alibaba Cloud(related: Alibaba Cloud support #17200), we could add oss_task_handler into alibaba-provider to enable remote-logging to OSS(alibaba cloud object storage service)
- Currently there's no python lib directly providing mocked oss to use in test, we may find a way to mock oss and add more unit tests later but not in this pr. It will probably take some more time. related: Switch Alibaba tests to use Mocks #17617
- This pr closes: Add oss_task_handler for alibaba provider #21748
|
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)
|
|
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
|
Some static checks/docs are failing. |
8ea71d6 to
1dc604f
Compare
I've fixed the checks/docs failures in the latest commit. Thx for the reminder. |
|
@mik-laj Could you please take a look at this pr? BTW, I will fix the unit tests in alibaba provider in a separate pr, using unittest.mock to replace the real external storage during unit test. THX |
|
You need to rebase @EricGao888 |
@potiuk Thanks for the reminder! I will rebase it, resolve the comments above and submit a new commit today. |
|
LGTM. @mik-laj ? |
|
@EricGao888 - just the few suggestions from @mik-laj and we can merge it :) |
|
|
Seems docs build failed. Not sure whether should I rebase again? Looks like related: #22100 |
|
@EricGao888 I rebased this PR. |