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

Fixes #16922 - Clean old tasks automaticaly #278

Merged
merged 8 commits into from Sep 21, 2017

Conversation

adamruzicka
Copy link
Contributor

This PR includes #247. Based on discussion in original PR this now also allows to distinguish between green an red (and any other) tasks and set different cleanup intervals.

@@ -16,11 +16,24 @@
:cleanup:
#
# the period after which to delete all the tasks (by default all tasks are not being deleted after some period)
# will be deprecated (TODO: mention when) and the use of rules is recommended
Copy link
Member

Choose a reason for hiding this comment

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

Whatever comes with Foreman 1.18?

@@ -11,11 +11,17 @@ def self.run(options)
end
end
if cleanup_settings[:after]
# TODO: Mention version in which this will be removed completely
ActiveSupport::Deprecation.warn('You are using legacy interface, please migrate from using :after to cleanup rules')
Copy link
Member

Choose a reason for hiding this comment

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

Foreman::Deprecation.deprecation_warning("1.18", _(':after setting in tasks cleanup section is deprecated, use :after in :rules section to set the value. to cleanup rules'))

@iNecas
Copy link
Member

iNecas commented Sep 19, 2017

Let's rebase this and add some tests before we proceed

@adamruzicka
Copy link
Contributor Author

The test matrix is all green but the build was marked as failed. Let's try it again

[test]

- :filter: result = success
:after: 30d
# Delete everything (any action, any state) after one year
- :states: []
Copy link
Member

Choose a reason for hiding this comment

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

I know I just keep adding comments here, but this is the last one (hopefully:). It's not very clear that empty states means all while missing/nil means 'stopped'. What about introducing 'all' as recognized value that would replace the [] optoin we have now.

@iNecas
Copy link
Member

iNecas commented Sep 21, 2017

Looks good, waiting for tests to finish. There is one failure so far, but seems not to be related directly

@adamruzicka
Copy link
Contributor Author

The tests are done, with one (most likely) unrelated failure.

@iNecas iNecas merged commit 488ea25 into theforeman:master Sep 21, 2017
@iNecas
Copy link
Member

iNecas commented Sep 21, 2017

Thanks @adamruzicka and @mbacovsky

@adamruzicka adamruzicka deleted the 16922_cleanup_cron branch September 21, 2017 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants