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

Add support for Microsoft Azure Blob Storage in Google Cloud Storage Transfer Service operators #8818

Closed
wants to merge 1 commit into from
Closed

Conversation

khyurri
Copy link
Contributor

@khyurri khyurri commented May 11, 2020

Operators Code, examples and documentation completed. I've got bug on CloudDataTransferServiceCancelOperationOperator from example. I'll fix it.

Can you, please, review changed code?

Make sure to mark the boxes below before creating PR: [x]

  • Description above provides context of the change
  • Unit tests coverage for changes (not needed for documentation changes)
  • Target Github ISSUE in description if exists
  • Commits follow "How to write a good git commit message"
  • Relevant documentation is updated including usage instructions.
  • I will engage committers as explained in Contribution Workflow Example.

In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.

@boring-cyborg boring-cyborg bot added the provider:google Google (including GCP) related issues label May 11, 2020
@khyurri khyurri changed the title WIP: Feature/azure transfer api WIP: Add support for Microsoft Azure Blob Storage in Google Cloud Storage Transfer Service operators May 11, 2020
@mik-laj
Copy link
Member

mik-laj commented Jun 4, 2020

@khyurri Do you need any help? I am available for you if you have any questions.

@khyurri
Copy link
Contributor Author

khyurri commented Jun 22, 2020

@mik-laj
yes, I think I need help with previewing documentation. I cannot build documentation locally and cannot check it is correct

@ad-m
Copy link
Contributor

ad-m commented Jun 22, 2020

@khyurri , how do you attempt to build documentation and what errors are you getting?

@potiuk
Copy link
Member

potiuk commented Jun 22, 2020

Indeed we have a super-easy way to build documentation - simply run './breeze build-documentation' and all should happen automatically. I would love to hear what problems you experience with it.

@khyurri
Copy link
Contributor Author

khyurri commented Jun 22, 2020

@ad-m
I build documentation using this command from repository root :

python setup.py build_sphinx

Build completes without any errors with message: The HTML pages are in build/sphinx/html.

I can find separate documentation pages (like in operators/cloud_storage_transfer_service/index.html), but cannot find example usages (like howto_operator_gcp_transfer_create_job_body_azure).

@potiuk
Copy link
Member

potiuk commented Jun 22, 2020

@khyurri - here is the official way of building the documentation (described in our CONTRIBUTING document):
https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#documentation

Additionally (if you do not want to setup your virtualenv) we have BREEZE development environment that downloads and uses Docker image to do the same: https://github.com/apache/airflow/blob/master/BREEZE.rst#using-the-airflow-breeze-environment

@khyurri
Copy link
Contributor Author

khyurri commented Jun 22, 2020

@potiuk @ad-m
I builded documentation locally, but I can't find example html for this file. airflow/providers/google/cloud/example_dags/example_cloud_storage_transfer_service_azure.py

How examples are displayed in the documentation?
As far as I understand this documentation is displayed here https://airflow.apache.org/docs/stable/howto/operator/gcp/transfer.html.
Is it enough for me to check that the examples from the documentation work, or should I do something else?

@mik-laj
Copy link
Member

mik-laj commented Jul 10, 2020

@khyurri Sorry. I missed your messsage. Example DAG files are not available in the documentation until they are used in the documentation. You can attach a fragment of files using the exampleinclude directive. Documentation for this service can be found here.
https://github.com/apache/airflow/blob/master/docs/howto/operator/gcp/cloud_storage_transfer_service.rst

Is it enough for me to check that the examples from the documentation work, or should I do something else?

I would be happy if you run example DAG and see if it works. You can do it with the help of automatic system tests.

I am very sad that you had to wait so long for my answer. I hope that my inattention will discourage you from completing this change.

@khyurri khyurri marked this pull request as ready for review August 3, 2020 09:03
@khyurri khyurri changed the title WIP: Add support for Microsoft Azure Blob Storage in Google Cloud Storage Transfer Service operators Add support for Microsoft Azure Blob Storage in Google Cloud Storage Transfer Service operators Aug 3, 2020
@khyurri
Copy link
Contributor Author

khyurri commented Aug 3, 2020

I think it's done.

@turbaszek
Copy link
Member

@khyurri can you rebase please? In the meantime we introduced black formatter for providers package. Sorry for such a long time of waiting for review.

CC: @michalslowikowski00 may be interested in this PR

@RosterIn
Copy link
Contributor

@khyurri seems like rebase wasn't successful

@khyurri
Copy link
Contributor Author

khyurri commented Oct 21, 2020

seems like rebase wasn't successful

Yeah. Working on it.

@mik-laj
Copy link
Member

mik-laj commented Oct 21, 2020

@khyurri Do you need help? I will gladly do it for you.

@khyurri
Copy link
Contributor Author

khyurri commented Dec 16, 2020

@mik-laj
Hey, sorry for delay. Could you please help me to rebase this PR?

@ad-m
Copy link
Contributor

ad-m commented Dec 16, 2020

@khyurri do you have experience in rebase?

/sparkle @mik-laj for supporting Apache Airflow project!

@mik-laj
Copy link
Member

mik-laj commented Dec 18, 2020

Hey, sorry for delay. Could you please help me to rebase this PR?

I was busy with Airflow 2.0, but today I will try to look at it.

@khyurri
Copy link
Contributor Author

khyurri commented Dec 18, 2020

I rebased on master, black changed too much code.

@github-actions
Copy link

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

# [END howto_operator_gcp_transfer_cancel_operation]

# [START howto_operator_gcp_transfer_delete_job]
delete_transfer_from_aws_job = CloudDataTransferServiceDeleteJobOperator(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
delete_transfer_from_aws_job = CloudDataTransferServiceDeleteJobOperator(
delete_transfer_from_job = CloudDataTransferServiceDeleteJobOperator(


# [START howto_operator_gcp_transfer_delete_job]
delete_transfer_from_aws_job = CloudDataTransferServiceDeleteJobOperator(
task_id="delete_transfer_from_aws_job",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
task_id="delete_transfer_from_aws_job",
task_id="delete_transfer_from_job",

create_transfer_job_from_azure >> wait_for_operation_to_start >> pause_operation
pause_operation >> list_operations >> get_operation >> resume_operation
resume_operation >> wait_for_operation_to_end >> cancel_operation
cancel_operation >> delete_transfer_from_aws_job
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cancel_operation >> delete_transfer_from_aws_job
cancel_operation >> delete_transfer_from_job

@mik-laj
Copy link
Member

mik-laj commented Dec 18, 2020

Code looks good to me. Can you add tests to avoid regression?

@khyurri
Copy link
Contributor Author

khyurri commented Dec 18, 2020

Sure

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Mar 19, 2021
@github-actions github-actions bot closed this Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:google Google (including GCP) related issues provider:microsoft-azure Azure-related issues stale Stale PRs per the .github/workflows/stale.yml policy file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants