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

[option2] Handle schedule computed fields no-ops, keep disabled next_run behavior #3936

Closed
wants to merge 1 commit into from

Conversation

AlanCoding
Copy link
Member

SUMMARY

Put simply, this is a more conservative version of #3933

Connect #3912

I had second thoughts about not updating next_run for disabled schedules. It shouldn't be that expensive, and I still believe that this change will be plenty sufficient to resolve the linked issue. This PR keeps the behavior where we update next_run for disabled schedules.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • API
AWX VERSION
4.0.0
ADDITIONAL INFORMATION

Continue practice of setting a next_run value for disabled schedules
Bail if no fields are changed
Do not update related template if its fields did not change
Keeping updates to disabled schedules, we still must
  call related template computed fields on every save
  because it's non-obvious if schedule was enabled / disabled

Change call pattern to schedule.update_computed_fields()
in doing so, fix bug where template does not pick up schedule
  due to schedules next_run not being saved

Handle the case (also a bug) where template was not updated
  when schedule was deleted
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

Copy link
Contributor

@ryanpetrello ryanpetrello left a comment

Choose a reason for hiding this comment

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

I like the other that is affected by disable=False; it seems more correct/accurate to me.

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