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

Allow optional end-date in get_execution_stats() #155

Merged
merged 6 commits into from
Sep 17, 2024

Conversation

IMC07
Copy link
Contributor

@IMC07 IMC07 commented Sep 13, 2024

Description

Add optional parameter of max end date for TaskDependcySensor get_execution_stats(). This is a minor change in order to not introduce a breaking change. However, if you agree I think it is more beautiful to have: 1) a function that returns the latest state of the run to be used for the 'poking' sensor, and have another 2) functionality that allows to query the API in a more flexible manner and allows to return more from the json response.
This way we are more flexible, so we can use the start/end/state details to trigger different paths in the Brickflow DAGs (with IfElseConditionTask tasks) for instance. E.g. if the Airflow dependency was finished before time x, then branch in this direction, otherwise y direction. Or if latest run had status 'failed', then branch in direction z etc.

Right now it could be used like this:

tds = TaskDependencySensor(
            task_id=job["dag_id"],
            airflow_auth=AirflowProxyOktaClusterAuth(
                oauth2_conn_id=oauth2_conn_id,
                airflow_cluster_url=job["cluster_url"],
                airflow_version="2.0.2",
            ),
            external_dag_id=job["dag_id"],
            external_task_id=job["task_id"],
            allowed_states=["success"]
        )

execution_date = parse("2024-09-10T04:00:00.000000+00:00")
task_status = tds.get_execution_stats(execution_date=execution_date, max_end_date=max_end_date)

Related Issue

#154

Motivation and Context

See issue/above.

How Has This Been Tested?

Tested locally and with unit tests.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@asingamaneni
Copy link
Collaborator

@IMC07 Looks like the format needs to be changed for the files

image

@IMC07
Copy link
Contributor Author

IMC07 commented Sep 17, 2024

@asingamaneni I forgot to run black, done now. Please check again.

asingamaneni
asingamaneni previously approved these changes Sep 17, 2024
Copy link
Collaborator

@asingamaneni asingamaneni left a comment

Choose a reason for hiding this comment

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

LGTM

@asingamaneni asingamaneni self-requested a review September 17, 2024 16:06
@asingamaneni
Copy link
Collaborator

Can you try to run make cov in your local and test it once again

Copy link
Collaborator

@asingamaneni asingamaneni left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution

@asingamaneni asingamaneni self-requested a review September 17, 2024 17:58
@asingamaneni asingamaneni merged commit 922953d into Nike-Inc:main Sep 17, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants