Skip to content

Conversation

@phanikumv
Copy link
Contributor

@phanikumv phanikumv commented Mar 13, 2023

This PR donates the deferrable AzureDataFactoryRunPipelineOperator from astronomer-providers to airflow repo.

cc @josh-fell


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@phanikumv phanikumv changed the title [WIP]: Add deferrable AzureDataFactoryRunPipelineOperator [WIP]: Add deferrable AzureDataFactoryRunPipelineOperator Mar 13, 2023
@phanikumv phanikumv force-pushed the azure_pipeline_asyncop branch from 702df16 to 4a84704 Compare March 14, 2023 02:13
@phanikumv phanikumv marked this pull request as ready for review March 14, 2023 06:48
@phanikumv phanikumv changed the title [WIP]: Add deferrable AzureDataFactoryRunPipelineOperator Add deferrable AzureDataFactoryRunPipelineOperator Mar 14, 2023
@josh-fell josh-fell self-requested a review March 14, 2023 12:20
Copy link
Contributor

Choose a reason for hiding this comment

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

There needs to be an empty between docstring description and parameter directives to make sure the params are rendered correctly in the Python API docs.

Comment on lines 108 to 118
Copy link
Contributor

Choose a reason for hiding this comment

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

AzureDataFactoryPipelineRunStatus already exists so let's use that. No problem augmenting the status groups too with INTERMEDIATE_STATUSES and FAILURE_STATUSES. I don't think SUCCESS_STATES is needed though since the logic could be pipeline_status == AzureDataFactoryPipelineRunStatus.SUCCESS. TERMINAL_STATUSES already exists in AzureDataFactoryPipelineRunStatus.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
:dedent: 0
:dedent: 4

So the code snippet doesn't have extra whitespace on its left side in the docs.

@phanikumv phanikumv closed this Mar 16, 2023
@phanikumv phanikumv force-pushed the azure_pipeline_asyncop branch from f6472c8 to 971e322 Compare March 16, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants