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 deferrable AzureDataFactoryRunPipelineOperator #30074

Closed
wants to merge 0 commits into from

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 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
@@ -30,7 +31,6 @@ class ADFPipelineRunStatusSensorTrigger(BaseTrigger):
"""
ADFPipelineRunStatusSensorTrigger is fired as deferred class with params to run the
task in trigger worker, when ADF Pipeline is running

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
QUEUED = "Queued"
IN_PROGRESS = "InProgress"
SUCCEEDED = "Succeeded"
FAILED = "Failed"
CANCELING = "Canceling"
CANCELLED = "Cancelled"

INTERMEDIATE_STATES: list[str] = [QUEUED, IN_PROGRESS, CANCELING]
FAILURE_STATES: list[str] = [FAILED, CANCELLED]
SUCCESS_STATES: list[str] = [SUCCEEDED]
TERMINAL_STATUSES: list[str] = [CANCELLED, FAILED, SUCCEEDED]
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.


.. exampleinclude:: /../../tests/system/providers/microsoft/azure/example_adf_run_pipeline.py
:language: python
:dedent: 0
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.

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.

None yet

3 participants