[WIP] [Issue#4939] Changes to support new purge task executions and purge workflows #4924

srimandaleeka01 · 2020-04-27T18:38:43Z

CLAassistant · 2020-04-27T18:38:49Z

All committers have signed the CLA.

…ri/pxpse_8278

nmaludy

@srimandaleeka01 I think overall these changes are great to see.

What i think would be a better approach though is to integrate the workflow and task purging into the already existing purge executions script. This way things like the st2garbagecollector process will be automatically updated.

Should be just moving some code around.

Does that sound good?

stale · 2020-08-27T01:05:56Z

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically marking is as stale. If this issue is not relevant or applicable anymore (problem has been fixed in a new version or similar), please close the issue or let us know so we can close it. On the contrary, if the issue is still relevant, there is nothing you need to do, but if you have any additional details or context which would help us when working on this issue, please include it as a comment to this issue.

…lector.

====================================================================== 1) ERROR: test_run (test_action_unload.UnloadActionTestCase) ---------------------------------------------------------------------- Traceback (most recent call last): tests/test_action_unload.py line 100 in test_run action.run(packs=[pack]) actions/pack_mgmt/unload.py line 68 in run self._unregister_rules(pack=pack) actions/pack_mgmt/unload.py line 99 in _unregister_rules cleanup_trigger_db_for_rule(rule_db=rule_db) /home/runner/work/st2/st2/st2common/st2common/services/triggers.py line 332 in cleanup_trigger_db_for_rule Trigger.delete_if_unreferenced(existing_trigger_db) /home/runner/work/st2/st2/st2common/st2common/persistence/trigger.py line 61 in delete_if_unreferenced cls._get_impl().delete_by_query(**delete_query) /home/runner/work/st2/st2/st2common/st2common/models/db/__init__.py line 479 in delete_by_query super(self) TypeError: super() argument 1 must be type, not MongoDBAccess

srimandaleeka01 · 2021-02-24T21:09:50Z

@srimandaleeka01 I think overall these changes are great to see.

What i think would be a better approach though is to integrate the workflow and task purging into the already existing purge executions script. This way things like the st2garbagecollector process will be automatically updated.

Should be just moving some code around.

Does that sound good?

Thanks for the pointer @nmaludy . Sorry have been out of loop on this project. I made the changes to call purge workflow and task executions in the garbagecollector . Please take a look : 36c7fdf

With the above change I also added 2 new attributes _workflow_execution_ttl _task_execution_ttl this is causing the unit tests to fail . How can I fix the unit test ? Couldn't figure out yet where the st2.conf file is being patched or where the fixture is

Thanks again!

----------------------------------------------------------------------
   Traceback (most recent call last):
    /opt/hostedtoolcache/Python/3.6.13/x64/lib/python3.6/site-packages/mock/mock.py line 1305 in patched
      return func(*args, **keywargs)
    st2actions/tests/unit/test_workflow_engine.py line 226 in test_process_error_handling_has_error
      gc = garbage_collector.GarbageCollectorService()
    st2reactor/st2reactor/garbage_collector/base.py line 72 in __init__
      self._workflow_execution_ttl = cfg.CONF.garbagecollector.workflow_execution_ttl
    /opt/hostedtoolcache/Python/3.6.13/x64/lib/python3.6/site-packages/oslo_config/cfg.py line 2547 in __getattr__
      return self._conf._get(name, self._group)
    /opt/hostedtoolcache/Python/3.6.13/x64/lib/python3.6/site-packages/oslo_config/cfg.py line 2264 in _get
      value = self._do_get(name, group, namespace)
    /opt/hostedtoolcache/Python/3.6.13/x64/lib/python3.6/site-packages/oslo_config/cfg.py line 2282 in _do_get
      info = self._get_opt_info(name, group)
    /opt/hostedtoolcache/Python/3.6.13/x64/lib/python3.6/site-packages/oslo_config/cfg.py line 2415 in _get_opt_info
      raise NoSuchOptError(opt_name, group)
   NoSuchOptError: no such option in group garbagecollector: workflow_execution_ttl```

and an additional Lint failure as well 

```echo "# Sample config which contains all the available options which the corresponding descriptions" > conf/st2.conf.sample;
echo "# Note: This file is automatically generated using tools/config_gen.py - DO NOT UPDATE MANUALLY" >> conf/st2.conf.sample
echo "" >> conf/st2.conf.sample
. virtualenv/bin/activate; python ./tools/config_gen.py >> conf/st2.conf.sample;
make[1]: Leaving directory '/home/runner/work/st2/st2'
diff conf/st2.conf.sample /tmp/st2.conf.sample.upstream || (echo "conf/st2.conf.sample hasn't been re-generated and committed. Please run \"make configgen\" and include and commit the generated file." && exit 1)
167a168,172
> # Workflow execution output objects (ones generated by action output streaming) older than this value (days) will be automatically deleted.
> workflow_execution_ttl = 7
> # Task execution output objects (ones generated by action output streaming) older than this value (days) will be automatically deleted.
> task_execution_ttl = 7
> ```

cognifloyd · 2021-03-07T05:58:33Z

We just reformatted the code with black. So, would you merge master into your branch (resolve any conflicts & reformat with black)? This should help resolve any linting errors. Thanks!

stale · 2021-06-09T02:43:07Z

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically marking is as stale. If this issue is not relevant or applicable anymore (problem has been fixed in a new version or similar), please close the issue or let us know so we can close it. On the contrary, if the issue is still relevant, there is nothing you need to do, but if you have any additional details or context which would help us when working on this issue, please include it as a comment to this issue.

cognifloyd · 2021-07-29T16:15:54Z

@srimandaleeka01 I'd really like to see this in v3.6.0 if possible. Do you have bandwidth to bring this up-to-date with master?

Both unit and lint failures are due to missing config options. You can add those here:

st2/st2reactor/st2reactor/garbage_collector/config.py

Lines 81 to 99 in 2a6ec4f

    
           ttl_opts = [ 
        
               cfg.IntOpt( 
        
                   "action_executions_ttl", 
        
                   default=None, 
        
                   help="Action executions and related objects (live actions, action output " 
        
                   "objects) older than this value (days) will be automatically deleted.", 
        
               ), 
        
               cfg.IntOpt( 
        
                   "action_executions_output_ttl", 
        
                   default=7, 
        
                   help="Action execution output objects (ones generated by action output " 
        
                   "streaming) older than this value (days) will be automatically deleted.", 
        
               ), 
        
               cfg.IntOpt( 
        
                   "trigger_instances_ttl", 
        
                   default=None, 
        
                   help="Trigger instances older than this value (days) will be automatically deleted.", 
        
               ), 
        
           ]

Sorry for the duplicate message. I was logged into the wrong github account. I just deleted the other message.

cognifloyd · 2021-07-29T16:21:11Z

Also, could you edit your PR description to include Closes: #4939? Thanks!

cognifloyd · 2021-10-02T06:58:42Z

@srimandaleeka01 Would you be willing to allow maintainers to edit your PR? If so, I can quickly merge in master and adjust the config options for you.

amanda11 · 2022-01-10T13:27:43Z

st2reactor/st2reactor/garbage_collector/base.py

+        the criteria defined in the config.
+        """
+        utc_now = get_datetime_utc_now()
+        timestamp = (utc_now - datetime.timedelta(days=self._action_executions_ttl))


Think this should be _workflow_execution_ttl instead of actions

Thank you @amanda11 I will make this change.

amanda11 · 2022-01-10T13:27:54Z

st2reactor/st2reactor/garbage_collector/base.py

+        the criteria defined in the config.
+        """
+        utc_now = get_datetime_utc_now()
+        timestamp = (utc_now - datetime.timedelta(days=self._action_executions_ttl))


Think this should be _task_execution_ttl instead of actions

Think this should be _task_execution_ttl instead of actions

Thank you @amanda11 will make this change.

srimandaleeka01 · 2022-01-10T14:43:55Z

@srimandaleeka01 Would you be willing to allow maintainers to edit your PR? If so, I can quickly merge in master and adjust the config options for you.

@cognifloyd Please go ahead and the maintainers can edit the PR for getting it merged. Thank you!

amanda11 · 2022-01-21T13:59:57Z

@cognifloyd I can update the PR, as I have taken this code made some slight changes and got it installed on a ST2 system. So happy to do update to master and merge those changes in.

amanda11 · 2022-01-21T14:15:47Z

@srimandaleeka01 Could you check if maintainers have access to this PR request, as I'm getting 403 error trying to push.

Please see: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork for info on how to allow maintainers to edit a PR that has been created from a fork.

srimandaleeka01 · 2022-01-21T15:20:28Z

@srimandaleeka01 Could you check if maintainers have access to this PR request, as I'm getting 403 error trying to push.

Please see: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork for info on how to allow maintainers to edit a PR that has been created from a fork.

I was following the article above to allow mainters to edit the PR, however I don't see that option.
It's likely that the PR has been created from a Plexxi:st2 ( a fork from master:st2) which is more restrictive ( intentionally ) .

Are there any specific changes apart from the comments you have posted for the PR ? If so please post the diff for the corresponding file , I can manually apply.

amanda11

Thanks so much for this PR. It will be really great to get this into the next release of StackStorm.

Just some minor other comments I found, in addition to fix the CircleCI then:

Need to run black on the files changed, so that it doesn't break the lint checks
Need to add the change to st2reactor/st2reactor/garbage_collector/config.py as mentioned earlier in review comments to prevent the UT failure to add the new config options.

Can you also update CHANGELOG.rst - to update the "Added" section to include details of this PR?

If need any assistance or want us to help with doing any of the amendments then just please let us know (we'd need the fork updated with permissions for maintainers to edit as I think at moment it's blocked). But if you are happy to do the changes there would be no need.

amanda11 · 2022-01-10T13:30:23Z

st2common/st2common/garbage_collection/workflows.py

+        logger.exception('Deletion of execution models failed for query with filters: %s.',
+                         exec_filters)
+    else:
+        logger.info('Deleted %s action execution objects' % deleted_count)


Probably best to change logger to workflow execution objects not "action execution objects", and other references to action in this method.

amanda11 · 2022-01-10T13:31:01Z

st2common/st2common/garbage_collection/workflows.py

+        logger.exception('Deletion of execution models failed for query with filters: %s.',
+                         exec_filters)
+    else:
+        logger.info('Deleted %s action execution objects' % deleted_count)


Probably best to change logger to task execution objects not "action execution objects", and other references to actiosn or action executions in this method.

amanda11 · 2022-01-21T15:19:49Z

st2common/st2common/cmd/purge_task_executions.py

@@ -0,0 +1,88 @@
+# Copyright 2019 Extreme Networks, Inc.


Copyright is now StackStorm Authors, and year file first introduced, e.g. could you change to:

Copyright 2021, The StackStorm Authors.

amanda11 · 2022-01-21T15:19:59Z

st2common/st2common/cmd/purge_workflows.py

@@ -0,0 +1,88 @@
+# Copyright 2019 Extreme Networks, Inc.


amanda11 · 2022-01-21T15:20:08Z

st2common/st2common/garbage_collection/workflows.py

@@ -0,0 +1,160 @@
+# Copyright 2020 Extreme Networks, Inc.


amanda11 · 2022-01-21T15:20:25Z

st2common/tests/unit/test_purge_task_executions.py

@@ -0,0 +1,72 @@
+# Copyright 2019 Extreme Networks, Inc.


amanda11 · 2022-01-21T15:20:33Z

st2common/tests/unit/test_purge_worklows.py

@@ -0,0 +1,72 @@
+# Copyright 2019 Extreme Networks, Inc.


amanda11 · 2022-01-21T15:23:04Z

st2common/tests/unit/test_purge_task_executions.py

+from datetime import timedelta
+
+from st2common import log as logging
+from st2common.constants.triggers import TRIGGER_INSTANCE_PROCESSED


The import of TRIGGER_INSTANCE_PROCESSED isn't used, so it breaks the CI checks.

amanda11 · 2022-01-21T15:26:07Z

st2common/tests/unit/test_purge_worklows.py

+from datetime import timedelta
+
+from st2common import log as logging
+from st2common.constants.triggers import TRIGGER_INSTANCE_PROCESSED


As TRIGGER_INSTANCE_PROCESSED is unused it will break the CI checks, so can you remove this import.

amanda11 · 2022-01-21T15:54:14Z

st2common/st2common/garbage_collection/workflows.py

+        filters['status'] = {'$in': DONE_STATES}
+
+    exec_filters = copy.copy(filters)
+    # TODO: Are these parameters valid.


At the moment the action_ref passed in from the standalone tool, doesn't take affect - as its not used in this class. I don't believe action_ref is valid for WorkflowExecution or TaskExecution, so I would remove the commented out lines and also the action_ref in the cmd/purge_....py classes.

amanda11 · 2022-01-21T16:21:43Z

@srimandaleeka01 Could you check if maintainers have access to this PR request, as I'm getting 403 error trying to push.
Please see: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork for info on how to allow maintainers to edit a PR that has been created from a fork.

I was following the article above to allow mainters to edit the PR, however I don't see that option. It's likely that the PR has been created from a Plexxi:st2 ( a fork from master:st2) which is more restrictive ( intentionally ) .

Are there any specific changes apart from the comments you have posted for the PR ? If so please post the diff for the corresponding file , I can manually apply.

Hi - I've listed in that last review I think all the changes that I was going to make - just a few minor things like copyright, black formatting etc, and then I had a look at the TODO you had left around action_ref, and checked the collection contents - so think safest to remove the action_ref that was in the TODO whether it was relevant or not.
There is also a merge conflict with master - but the instructions to fix that are provided by GIT where it says there are conflicts (it's just that in setup.py on master it has a new binary in its list, so just a small conflict on that to change).

amanda11 · 2022-03-04T11:56:53Z

@srimandaleeka01 Are you ok to sign the CLA Contribution Agreement? At the moment it is blocking the PR as the contribution agreement isn't signed.
We really appreciate this fix, and we'd like to get it into the 3.7.0 release, which is targetted for end of March.

Do you have time to address the review comments this month, as we'd like to get this into the 3.7.0 PR. Alternatively, if the contribution agreement is signed, then we could fork off this repo and then merge a PR that way - and ask one of the TSC to make the review comments? Whichever you prefer.

Thanks once again for the contribution.

changes to support new purge task executions and st2 purge workflow

66a06e9

pull-request-size bot added the size/S PR that changes 10-29 lines. Very easy to review. label Apr 27, 2020

srimandaleeka01 and others added 3 commits April 27, 2020 14:39

Merge branch 'master' into sri/pxpse_8278

4ee1b5d

Support new purge task executions and st2 purge workflow

5f1dab8

Merge branch 'sri/pxpse_8278' of https://github.com/Plexxi/st2 into s…

249eebd

…ri/pxpse_8278

pull-request-size bot added size/XL PR that changes 500-999 lines. Consider splitting work into several ones that easier to review. and removed size/S PR that changes 10-29 lines. Very easy to review. labels Apr 28, 2020

Merge branch 'master' into sri/pxpse_8278

62dd9ac

srimandaleeka01 changed the title ~~changes to support new purge task executions and st2 purge workflow~~ [WIP] Changes to support new purge task executions and st2 purge workflow Apr 28, 2020

srimandaleeka01 mentioned this pull request May 6, 2020

Support purge operations for st2 Workflows and TaskExecution #4939

Closed

srimandaleeka01 changed the title ~~[WIP] Changes to support new purge task executions and st2 purge workflow~~ [WIP] [Issue#4939] Changes to support new purge task executions and st2 purge workflow May 6, 2020

Merge branch 'master' into sri/pxpse_8278

cb28924

nmaludy requested a review from m4dcoder May 26, 2020 14:58

nmaludy suggested changes May 26, 2020

View reviewed changes

nmaludy linked an issue May 26, 2020 that may be closed by this pull request

Support purge operations for st2 Workflows and TaskExecution #4939

Closed

stale bot added the stale label Aug 27, 2020

Merge branch 'master' into sri/pxpse_8278

95d0cf3

stale bot removed the stale label Feb 24, 2021

srimandaleeka01 added 2 commits February 24, 2021 14:33

Call the purge workflow and task execution methods in the garbage col…

36c7fdf

…lector.

srimandaleeka01 requested a review from nmaludy February 24, 2021 21:19

srimandaleeka01 changed the title ~~[WIP] [Issue#4939] Changes to support new purge task executions and st2 purge workflow~~ [WIP] [Issue#4939] Changes to support new purge task executions and purge workflows Feb 24, 2021

stale bot added the stale label Jun 9, 2021

stale bot removed the stale label Jul 29, 2021

cognifloyd added database feature service: workflow engine status:needs changes workflows: orquesta labels Jul 29, 2021

cognifloyd added this to the 3.6.0 milestone Jul 29, 2021

arm4b modified the milestones: 3.6.0, 3.7.0 Oct 5, 2021

arm4b added this to In progress in StackStorm v3.7.0 via automation Oct 5, 2021

arm4b removed this from In progress in StackStorm v3.7.0 Jan 6, 2022

arm4b removed this from the 3.7.0 milestone Jan 6, 2022

arm4b mentioned this pull request Jan 6, 2022

ST2 garbage collector is not cleaning up records in task_execution_d_b #5525

Closed

amanda11 added this to the 3.7.0 milestone Jan 6, 2022

amanda11 added this to In progress in StackStorm v3.7.0 via automation Jan 6, 2022

amanda11 requested changes Jan 10, 2022

View reviewed changes

amanda11 requested changes Jan 21, 2022

View reviewed changes

amanda11 added a commit that referenced this pull request Apr 1, 2022

Merge of #4924 from Plexxi/st2 sri/pxpse_8278 branch

a49a1d7

amanda11 mentioned this pull request Apr 1, 2022

Changes to support new purge task executions and purge workflows #5605

Merged

cognifloyd closed this in #5605 Apr 1, 2022

StackStorm v3.7.0 automation moved this from In progress to Done Apr 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] [Issue#4939] Changes to support new purge task executions and purge workflows #4924

[WIP] [Issue#4939] Changes to support new purge task executions and purge workflows #4924

srimandaleeka01 commented Apr 27, 2020 •

edited by arm4b

CLAassistant commented Apr 27, 2020 •

edited

nmaludy left a comment

stale bot commented Aug 27, 2020

srimandaleeka01 commented Feb 24, 2021 •

edited

cognifloyd commented Mar 7, 2021

stale bot commented Jun 9, 2021

cognifloyd commented Jul 29, 2021

cognifloyd commented Jul 29, 2021

cognifloyd commented Oct 2, 2021

amanda11 Jan 10, 2022

srimandaleeka01 Jan 21, 2022

amanda11 Jan 10, 2022

srimandaleeka01 Jan 21, 2022

srimandaleeka01 commented Jan 10, 2022 •

edited

amanda11 commented Jan 21, 2022

amanda11 commented Jan 21, 2022

srimandaleeka01 commented Jan 21, 2022 •

edited

amanda11 left a comment

amanda11 Jan 10, 2022

amanda11 Jan 10, 2022

amanda11 Jan 21, 2022

amanda11 Jan 21, 2022

amanda11 Jan 21, 2022

amanda11 Jan 21, 2022

amanda11 Jan 21, 2022

amanda11 Jan 21, 2022

amanda11 Jan 21, 2022

amanda11 Jan 21, 2022

amanda11 commented Jan 21, 2022

amanda11 commented Mar 4, 2022

[WIP] [Issue#4939] Changes to support new purge task executions and purge workflows #4924

[WIP] [Issue#4939] Changes to support new purge task executions and purge workflows #4924

Conversation

srimandaleeka01 commented Apr 27, 2020 • edited by arm4b

CLAassistant commented Apr 27, 2020 • edited

nmaludy left a comment

Choose a reason for hiding this comment

stale bot commented Aug 27, 2020

srimandaleeka01 commented Feb 24, 2021 • edited

cognifloyd commented Mar 7, 2021

stale bot commented Jun 9, 2021

cognifloyd commented Jul 29, 2021

cognifloyd commented Jul 29, 2021

cognifloyd commented Oct 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srimandaleeka01 commented Jan 10, 2022 • edited

amanda11 commented Jan 21, 2022

amanda11 commented Jan 21, 2022

srimandaleeka01 commented Jan 21, 2022 • edited

amanda11 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Copyright 2021, The StackStorm Authors.

Choose a reason for hiding this comment

Copyright 2021, The StackStorm Authors.

Choose a reason for hiding this comment

Copyright 2021, The StackStorm Authors.

Choose a reason for hiding this comment

Copyright 2021, The StackStorm Authors.

Choose a reason for hiding this comment

Copyright 2021, The StackStorm Authors.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amanda11 commented Jan 21, 2022

amanda11 commented Mar 4, 2022

srimandaleeka01 commented Apr 27, 2020 •

edited by arm4b

CLAassistant commented Apr 27, 2020 •

edited

srimandaleeka01 commented Feb 24, 2021 •

edited

srimandaleeka01 commented Jan 10, 2022 •

edited

srimandaleeka01 commented Jan 21, 2022 •

edited