-
Notifications
You must be signed in to change notification settings - Fork 94
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
main loop: plugins for periodic functions #3492
Conversation
697b06e
to
2ba27c7
Compare
9a87123
to
5bd3fd1
Compare
a810790
to
0292644
Compare
0292644
to
186cfb5
Compare
This is great 👍 Will have a closer look soon.. Does the memory profile exclude recursive object references? |
Yes, Pympler only counts objects once. A simple example: >>> print(asized([1, 2, 1], detail=2).format())
[1, 2, 1] size=152 flat=88
1 size=32 flat=32
2 size=32 flat=32
1 size=0 flat=0 A recursive example: >>> a = [1, 2, 3]
>>> b = [4, 5, 6]
>>> a.append(a)
>>> b.append(a)
>>> print(asized(b, detail=4).format())
[4, 5, 6, [1, 2, 3, [...]]] size=432 flat=120
[1, 2, 3, [...]] size=216 flat=120
1 size=32 flat=32
2 size=32 flat=32
3 size=32 flat=32
[1, 2, 3, [...]] size=0 flat=0 # the recursive obj has a size of 0
4 size=32 flat=32
5 size=32 flat=32
6 size=32 flat=32 (note recursive objects have a size of 0 because the pointer overheads belong to the However beware, Pympler only counts memory once, but it does so the first time it encounters an object which means you can get very different results for something which is fundamentally the same measurement: >>> a = list(range(1000))
>>> b = ['x', 'y', a]
>>> c = ['z', a]
The list `b` is huge but `c` is tiny:
>>> print(asized([b, c], detail=1).format())
[['x', 'y', [0, 1, 2, 3, 4, 5, 6, 7, 8...., 993, 994, 995, 996, 997, 998, 999]]] size=41520 flat=80
['x', 'y', [0, 1, 2, 3, 4, 5, 6, 7, 8,....2, 993, 994, 995, 996, 997, 998, 999]] size=41304 flat=88
['z', [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 1....2, 993, 994, 995, 996, 997, 998, 999]] size=136 flat=80
Wait, what, now it's the other way around:
>>> print(asized([c, b], detail=1).format())
[['z', [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, ...., 993, 994, 995, 996, 997, 998, 999]]] size=41520 flat=80
['z', [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 1....2, 993, 994, 995, 996, 997, 998, 999]] size=41240 flat=80
['x', 'y', [0, 1, 2, 3, 4, 5, 6, 7, 8,....2, 993, 994, 995, 996, 997, 998, 999]] size=200 flat=88 In Cylc the main offender for this is the config object which will be associated with whatever object references it first. |
Adding the WIP label as this should to be generalised in light of the desired async main-loop endgame. |
e4a1100
to
d9f1106
Compare
Note:
Remaining test failure turns out to be a faff, test suite behaves differently when run outside the battery. Note: issue probably triggered by health check interval |
Here is an overview of what got changed by this pull request: Issues
======
+ Solved 2
Complexity increasing per file
==============================
- cylc/flow/main_loop/log_memory.py 6
- cylc/flow/main_loop/log_data_store.py 7
- cylc/flow/main_loop/__init__.py 7
- cylc/flow/main_loop/health_check.py 3
- cylc/flow/host_select.py 15
- cylc/flow/main_loop/log_main_loop.py 5
- cylc/flow/main_loop/auto_restart.py 10
- cylc/flow/parsec/OrderedDict.py 8
- cylc/flow/scripts/cylc_psutil.py 4
Complexity decreasing per file
==============================
+ cylc/flow/scheduler_cli.py -1
See the complete overview on Codacy |
da9a540
to
e25ae17
Compare
Ok, I've added decorator functions for writing plugins: from cylc.flow.main_loop import periodic
@periodic
def monitor_something(sched, state): pass This should be forward compatible with the sort of stuff we may want to do later: from cylc.flow.main_loop import task_event
@task_event
def monitor_task_event(task_event, state): pass |
e25ae17
to
84fcfce
Compare
Deconfilcted. |
84fcfce
to
0e6ce35
Compare
Rebased (should remove unit test failure)... |
@dwsutherland tagged you as a reviewer as you'd commented before, feel free to reassign if swamped by ops work, the graphql stuff is more important. |
0e6ce35
to
cecce72
Compare
Deconflicted. (plus some bonus documentation) |
9f4d970
to
7e69fe0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oliver-sanders, would it be possible to list the available plugins too?
For example, I was playing with the my_plugin
from the docs, but couldn't figure out which key/name to use. It would be helpful to get a list of available plugins in the error message.
2020-04-07T10:46:32+12:00 ERROR - No main-loop plugin: "myplugin"
Traceback (most recent call last):
File "/home/kinow/Development/python/workspace/cylc-flow/cylc/flow/main_loop/__init__.py", line 325, in load
module_name = entry_points[plugin_name.replace(' ', '_')]
KeyError: 'myplugin'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/kinow/Development/python/workspace/cylc-flow/cylc/flow/scheduler.py", line 306, in start
self.configure()
File "/home/kinow/Development/python/workspace/cylc-flow/cylc/flow/scheduler.py", line 558, in configure
self.options.main_loop
File "/home/kinow/Development/python/workspace/cylc-flow/cylc/flow/main_loop/__init__.py", line 327, in load
raise UserInputError(f'No main-loop plugin: "{plugin_name}"')
cylc.flow.exceptions.UserInputError: No main-loop plugin: "myplugin"
2020-04-07T10:46:32+12:00 CRITICAL - Suite shutting down - No main-loop plugin: "myplugin"
Tested the log data store and health check plugins with the following added to my ~/.cylc/flow/8.0a1/flow.rc
:
[cylc]
[[main loop]]
plugins = health check, log data store
[[[health check]]]
interval = PT10S
[[[log data store]]]
interval = PT10S
[[[my plugin]]]
Then setting breakpoints and looking at the console to confirm the code was executed.
Finally tested raising a ValueError
in health check and confirmed that log data store was still executed successfully.
🎉 🚀
I couldn't use the my-plugin
from the docs. Not sure what I am doing wrong? (left a comment in the code about it)
.. code-block:: console | ||
|
||
$ # run a workflow using the "health check" and "auto restart" plugins: | ||
$ cylc run my-workflow --main-loop 'health check' \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, couldn't work out how to get around that without the docstring going over 79 chars.
additional_plugins = additional_plugins or [] | ||
entry_points = pkg_resources.get_entry_map( | ||
'cylc-flow' | ||
).get('main_loop', {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't use the my-plugin
from the documentation. Debugging, the entry_points
here never had my my-plugin
, so that's never loaded correctly for me.
Is there anything I am missing @oliver-sanders ? I thought we would need something like this https://packaging.python.org/guides/creating-and-discovering-plugins/ (see part about namespaces)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(in reply to @kinow and this comment from @dwsutherland)
Maybe there's a way of doing it without adding to setup.cfg?
I haven't yet implemented the mechanism required for external projects to add entrypoints to Cylc Flow. Not really sure how to go about that, so for now I've documented the entry point as cylc.main_loop
.
The only experience I have with this sort of thing is with Pytest, which uses the entrypoint pytest11
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move that for later than 👍
Interesting new dependency added to Cylc Flow (optional I think for some plugins): https://pypi.org/project/kiwisolver/. Not sure where the name comes from, but the authors call it Kiwi. And quite sure one of the authors is the creator of Phosphor.JS (now Lumino). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good to me 👍
I managed to get the my-plugin
example working:
(cylc-doc) sutherlander@cortex-vbox:my_plugin$ more my_plugin.py
from cylc.flow import LOG
from cylc.flow.main_loop import startup
@startup
async def my_startup_coroutine(schd, state):
# write Hello <suite name> to the Cylc log.
LOG.info(f'Hello {schd.suite}')
(cylc-doc) sutherlander@cortex-vbox:my_plugin$ more setup.py
#!/usr/bin/env python
from setuptools import setup
# plugins must be properly installed, in-place PYTHONPATH meddling will
# not work.
setup(
name='my-plugin',
version='1.0',
py_modules=['my_plugin'],
entry_points={
# register this plugin with Cylc
'cylc.main_loop': [
# name = python.namespace.of.module
'my_plugin=my_plugin.my_plugin'
]
}
)
(cylc-doc) sutherlander@cortex-vbox:my_plugin$ pip install -e .^C
Now plugin available:
(cylc-doc) sutherlander@cortex-vbox:cylc-doc$ python
Python 3.7.5 (default, Nov 20 2019, 09:21:52)
[GCC 9.2.1 20191008] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import my_plugin
>>> dir(my_plugin)
['LOG', '__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__spec__', 'my_startup_coroutine', 'startup']
>>>
(cylc-doc) sutherlander@cortex-vbox:my_plugin$ cd ../cylc-flow/
(cylc-doc) sutherlander@cortex-vbox:cylc-flow$ git diff
diff --git a/setup.cfg b/setup.cfg
index f745f0d01..9f0ca49ed 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -145,3 +145,4 @@ main_loop =
log_data_store = cylc.flow.main_loop.log_data_store
log_main_loop = cylc.flow.main_loop.log_main_loop
log_memory = cylc.flow.main_loop.log_memory
+ my_plugin = my_plugin
(cylc-doc) sutherlander@cortex-vbox:cylc-flow$ pip install -e .["all"]^C
Maybe there's a way of doing it without adding to setup.cfg
?
(otherwise should probably be in the docs... Thought that was the point of the above entry_points
)
(cylc-doc) sutherlander@cortex-vbox:cylc-doc$ cylc run baz --main-loop 'my plugin'
._.
| | The Cylc Suite Engine [8.0a1]
._____._. ._| |_____. Copyright (C) 2008-2019 NIWA
| .___| | | | | .___| & British Crown (Met Office) & Contributors.
| !___| !_! | | !___. _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
!_____!___. |_!_____! This program comes with ABSOLUTELY NO WARRANTY.
.___! | It is free software, you are welcome to
!_____! redistribute it under certain conditions;
see `COPYING' in the Cylc source distribution.
*** listening on tcp://cortex-vbox:43015/ ***
*** publishing on tcp://cortex-vbox:43029 ***
To view suite server program contact information:
$ cylc get-suite-contact baz
Other ways to see if the suite is still running:
$ cylc scan -n 'baz' cortex-vbox
$ cylc ping -v --host=cortex-vbox baz
$ ps -wopid,args 25730 # on cortex-vbox
(cylc-doc) sutherlander@cortex-vbox:cylc-doc$ grep "Hello baz" /home/sutherlander/cylc-run/baz/log/suite/log
2020-04-07T16:46:01+12:00 INFO - Hello baz
When do we want to merge this?
That's what I thought. That users would need to just install the |
I think we can merge it once @oliver-sanders has address the question about how to add a plugin. I also thought |
Good idea. Done.
|
Yes I saw that! Wonderful Aus Vs NZ banter there. This is dragged in by matplotlib, which is an optional dependency that wouldn't get installed for most uses. |
Great reviewing BTW, it's a big help. |
@@ -42,6 +42,9 @@ | |||
|
|||
Set a sensible interval before running suites. | |||
|
|||
If ``matplotlib`` is installed this plugin will plot results as a PDF in | |||
the run directory when the suite is shut down (cleanly). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That cleanly note will be helpful too. I stopped the suite a few times with CTRL+C and went looking for the plot, then tried cylc stop five
and that worked.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should add a suite abort hook too at some point.
Will check out code and re-run tests tomorrow morning. That should give time to others to have a play if they'd like too. Really good stuff coming with this PR! We need to post that somewhere like Discourse, some twitter, blog, etc. This is bringing plugins to Cylc in a way that I bet users will find useful and enjoy using. |
Shouldn't |
There was already a dependency between cylc-doc and cylc-flow. This dependency isn't in the setup.py on-purpose because you never (other than perhaps CI at release time) want pip to install cylc-flow for you when building cylc docs. Less automatic but makes you think about what you are actually documenting. |
2 approvals, LGTM 🎉 |
Closes #3485
Redefines #2866 -> #3495
Facilitates #3329
Note: built atop #3489
Note: This is a first step, the interface may get generalised later. We may as well run all main loop functions through this interface (for logging, timing, etc).
Highlights
logging.DEBUG
level).--main-loop
arg.cylc.flow.scheduler.Scheduler
attributes.cylc.flow.scheduler
down a bit - this module has got too bloated.Caveats:
cfgspec
modules have been refactored in build documentation intocylc.flow.cfgspec
modules #3253 .Examples:
Changes to make to your global config for testing:
How to run a suite using extra plugins:
$ cylc run <suite> --main-loop 'log memory' --main-loop 'log data store'
How to write your own plugins:
Example plot from "log memory" (find it in the suite run dir after shutdown):
Example plot from "log main loop" showing the execution times of different plugins:
TODO:
Convert suite timeout(can do this whenever)Convert suite stall(can do this whenever)(awaiting cursory approval)
Requirements check-list
CONTRIBUTING.md
and added my name as a Code Contributor.