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

Deprecate SageMakerTrainingPrintLogTrigger #41158

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

vincbeck
Copy link
Contributor

This trigger is supposed to do the same job as SageMakerTrigger but with printing logs. The reasons to deprecate it:

  • Printing logs in a trigger does not add much value to the DAG author since logs go to the triggerer and not task logs
  • The trigger SageMakerTrainingPrintLogTrigger contains some bugs since the operator SageMakerTrainingOperator does not work when created with deferrable=True and print_log=True. The trigger wait indefinitely for the job to complete
  • Having two different triggers to achieve the same thing with the only difference one is pushing more logs than the other does not make a lot of sense

^ 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.

@boring-cyborg boring-cyborg bot added area:providers provider:amazon AWS/Amazon - related issues labels Jul 31, 2024
@vincbeck vincbeck merged commit ab0cf2e into apache:main Jul 31, 2024
52 checks passed
@vincbeck vincbeck deleted the vincbeck/sagemaker-trigger branch July 31, 2024 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:amazon AWS/Amazon - related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants