Skip to content
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

[AIRFLOW-17200] Add Alibaba Cloud OSS support #17201

Merged
merged 3 commits into from
Aug 7, 2021

Conversation

Gabriel39
Copy link
Contributor

Alibaba Cloud Object Storage Service (OSS) is an encrypted, secure, cost-effective, and easy-to-use object storage service that enables you to store, back up, and archive large amounts of data in the cloud, with a guaranteed durability of 99.9999999999%(12 9’s). (refer to https://www.alibabacloud.com/product/oss)

This PR is to enable Airflow users using OSS.

@Gabriel39
Copy link
Contributor Author

FYI: This PR is WIP. For now, OSS support is almost done except documents, and I will follow up soon. So feel free to comment for current work. Thx.

@Gabriel39
Copy link
Contributor Author

cc @potiuk @kaxil Thx

@eladkal eladkal linked an issue Jul 25, 2021 that may be closed by this pull request
@Gabriel39 Gabriel39 force-pushed the support_alibabacloud branch 2 times, most recently from 7ffd2c1 to ad08cdd Compare July 25, 2021 07:42
@Gabriel39 Gabriel39 changed the title [AIRFLOW-17200][WIP] Add Alibaba Cloud OSS support [AIRFLOW-17200] Add Alibaba Cloud OSS support Jul 25, 2021
@mik-laj
Copy link
Member

mik-laj commented Jul 27, 2021

What do you think about adding system tests to these operators?

@Gabriel39
Copy link
Contributor Author

What do you think about adding system tests to these operators?

Hi @mik-laj , I� updated my code and resubmitted this PR. Thank you for your advises!

@Gabriel39
Copy link
Contributor Author

cc: @vikramkoka thx :)

CONTRIBUTING.rst Outdated Show resolved Hide resolved
@Gabriel39
Copy link
Contributor Author

@vikramkoka @mik-laj need review. kind remind. Thx :)

@Gabriel39 Gabriel39 force-pushed the support_alibabacloud branch 3 times, most recently from f8930b0 to 6bc3460 Compare August 3, 2021 02:44
@Gabriel39 Gabriel39 force-pushed the support_alibabacloud branch 14 times, most recently from 5b1abc5 to a842146 Compare August 5, 2021 09:43
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fantastic. Just one small NIT - the provider version should be 1.0.0

airflow/providers/alibaba/CHANGELOG.rst Outdated Show resolved Hide resolved
airflow/providers/alibaba/provider.yaml Outdated Show resolved Hide resolved
Gabriel39 and others added 2 commits August 7, 2021 22:11
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
@Gabriel39
Copy link
Contributor Author

Looks fantastic. Just one small NIT - the provider version should be 1.0.0

@potiuk Updated. Thx :)

@Gabriel39 Gabriel39 requested a review from potiuk August 7, 2021 14:13
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Great job @Gabriel39 !

@github-actions
Copy link

github-actions bot commented Aug 7, 2021

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.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Aug 7, 2021
@potiuk potiuk merged commit 75ca654 into apache:main Aug 7, 2021
@ashb
Copy link
Member

ashb commented Aug 11, 2021

This makes every (or almost every) test run try to reach out to alibaba cloud in the oss-cn-hangzhou region, and is currently failing builds on main https://github.com/apache/airflow/runs/3290445260

  E               Failed: Timeout >60.0s
  /usr/local/lib/python3.6/site-packages/urllib3/util/connection.py:86: Failed

From what I can tell this bucket region is unreachable outside China, which means in the best case these tests will be skipped, and in the worst case as now they will be skipped.

I think we have to revert this PR until we have a fix for this. Or at the very least we will skip these tests and we can't release this provider until the tests are actually running.

ashb added a commit that referenced this pull request Aug 11, 2021
@potiuk
Copy link
Member

potiuk commented Aug 14, 2021

#17616 should fix the instability and #17617 is longer-term support to use mocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers full tests needed We need to run full set of tests for this PR to merge kind:documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alibaba Cloud support
6 participants