document suite runtime interface #3004
Conversation
oliver-sanders
force-pushed
the
zmq-document-interface
branch
from
fd4a416
to
452389b
Compare
There was a problem hiding this comment.
Thanks a ton for clarifying what methods are public and private in the API (i.e. renaming some methods to have the _ prefix). As well as for adding TODO markers so we remember to revisit parts of the code later.
Generated documentation locally with no issues. Used a Notebook to help me testing the code too, and it works (with a few hiccups, but works nicely).
Found some typos, but I think no blockers. So happy to approve once updated 👍
| @@ -235,14 +242,42 @@ def _authorise(self, *args, user='?', meta=None, **kwargs): | |||
| LOG.info( | |||
| '[client-command] %s %s@%s:%s', fcn.__name__, user, host, prog) | |||
| return fcn(self, *args, **kwargs) | |||
| _authorise.__doc__ += ( # add auth level to docstring | |||
| 'Authentication:\n%s:py:obj:`cylc.network.%s`\n' % ( | |||
| ' ' * 12, req_priv_level)) | |||
There was a problem hiding this comment.
Maybe now we start using the (supposedly faster, but maybe simpler) f-strings? They were introduced in Py3.6, which I think is our minimum version in Travis-CI?
_authorise.__doc__ += f"Authentication:\n{' ' * 12}:py:obj:cylc.network.{req_priv_level}\n" should work I believe.
(though that could be done later, incrementally, etc, feel free to resolve this conversation if you prefer 👍 )
There was a problem hiding this comment.
Might as well use the latest and greatest.
There was a problem hiding this comment.
but maybe simpler
Can do, I have mixed feelings about fstrings, Python has created an entire templating engine. From playing about with them so far, they have irritating limitations.
supposedly faster
Wow, really. That's surprising!
There was a problem hiding this comment.
_authorise.doc += f"Authentication:\n{' ' * 12}:py:obj:cylc.network.{req_priv_level}\n" should work I believe.
SyntaxError: f-string: expecting '}'
There was a problem hiding this comment.
I think you are using Py34, but minimum requirement for f-strings is Py36?
Py37:
Py36:
Py34:
There was a problem hiding this comment.
However, once running sphinx-build, it complained about the final value.
...
writing output... [100%] index
/home/kinow/Development/python/workspace/cylc/lib/cylc/network/server.py:docstring of cylc.network.server.SuiteRuntimeServer.api:15: WARNING: py:obj reference target not found: cylc.network.1
/home/kinow/Development/python/workspace/cylc/lib/cylc/network/server.py:docstring of cylc.network.server.SuiteRuntimeServer.clear_broadcast:31: WARNING: py:obj reference target not found: cylc.network.6
...
Had to change to
_authorise.__doc__ += ( # add auth level to docstring
f"Authentication:\n{' ' * 12}:py:obj:`cylc.network.Priv.{req_priv_level.name}`\n")
To get it working OK.
oliver-sanders
force-pushed
the
zmq-document-interface
branch
from
452389b
to
29a3f82
Compare
|
|
oliver-sanders
force-pushed
the
zmq-document-interface
branch
from
29a3f82
to
64674e1
Compare
|
FYI: The functional tests are now intermittently timing out. |
Kicked both builds that failed, should be green in a moment 👍 |
|
Hmmm, there is one build that is simply refusing to work. Kicked it four times today already.
I wonder if something changed in Travis configuration. We had a way around this default 10 minutes-no-response-killer feature from Travis I think? |
|
Travis CI is pretty frustrating at the moment. I don't recall if we had a solution for the timeout issue before though ... maybe not. |
|
The test which hangs seems to change around a bit, is it; Cylc hanging; a problem in my branch; Travis issues, I have no idea. We shouldn't have tests hanging. |
|
No, we shouldn't! (Or, yes, we shouldn't ... i.e. I'm agreeing!) |
By checking against the chunking algorithm the three tests hanging in that job are:
In my environment each passes individually and they all pass when run together in parallel.
I guess it could be, I can't work out what in this PR could cause it. |
|
@oliver-sanders so there was actually some discussion around tests being killed by Travis after an inactivity period, but it was in I tried to apply the same fix here, to see if that would fix it. My hunch was that perhaps the improvements in logging and exceptions, or maybe something in py3, could have caused our tests to be more silent. But that did not work. See Travis result here: https://travis-ci.org/kinow/cylc/builds/511280533 Builds were killed after 50 minutes of inactivity (which is a lot more than our normal 20-30 minutes). Looks like the tests are simply getting stuck somewhere. It should be able to reproduce locally. I'll try to run one of the test chunks that failed in my computer to see if that works or not too 👍 |
|
Maybe this could be helpful:
Looks like.... |
|
Tried to find what was hanging, but all that I could find running I think whatever crashed sleep had already been killed. So looked at And I tried running all tests mentioned here, and all of these tests passed when executed isolated. My guess now is that there could be a problem when they are executed in parallel, or maybe with our test or coverage tools. Sorry if not really helpful 😞 |
|
That is interesting/weird. I'm pretty sure I've seen several "sleep crashed" dialogs on my Ubuntu 18.04 VM lately too. |
|
I had noticed it once a little while ago, while running some command in Cylc. But I simply ignored it and continued doing whatever I was doing at that moment. I wonder why this pull request in special is failing. |
|
Looks like one of these occurred recently on my system: |
|
Yeah I had a couple of Ubuntu sleep crashes while working on that flask-graphql branch a while ago.. |
|
The test job mentioned above ( Doing this manually doesn't cause |
That may be a really good lead @dwsutherland . Does that mean you had this issue before the Python 3 changes? Could you check tomorrow if you also have one of these log files? I assume you were using Linux? If not, Windows Event Viewer may have something useful (though I have no idea if Cylc runs on Windows/cygwin/subsystem/etc). |
We definitely don't run on Windows itself. Cygwin or Linux on "windows subsystem for Linux" probably, but I haven't tried yet (I think @dpmatthews said he did that - successfully - not so long ago?). |
Nope - not me |
OK, god knows why I thought that. |
|
I tried Cylc on WSL some time ago with limited success. It works with a very basic suite, but had issues as soon as the suite is a bit more involved. I'm sure things have improved recently. Cylc works perfectly on the Chromebook, however. |
|
Executed the same CHUNK=4/4 locally today, after restarting my virtual machine. It started around 10AM, and has been running since then (13:07 now). It is stuck, without a crash. I did a Will poke around a bit with |
|
What the parent process is up to:
The Which is also waiting, but on any process (the And the Cylc suites: Looks like they are all waiting on file descriptors. I think the four child processes are suffering from the same illness. So will diagnose one only ( Before doing any more digging, looking at the log files of the job, Sook looks like it is still running. I waited until EDIT: but my first attempt started without much luck... |
|
Best I could find was this suite has the following in its sqlite DB:
Not sure why the |
Because the "hello" task never finished (or even started, by the look of it - it's in the "submitted" state ... so the first question is, why is that?) |
|
Shall we move the test battery investigation to #3046 ? (it's a bit off-topic here). |
|
All checks pass (thanks @oliver-sanders for looking into the issue on Travis). Keeping my +1 for merging it. 🎉 |
oliver-sanders
force-pushed
the
zmq-document-interface
branch
from
d649ad1
to
0e4ce21
Compare
Follow on from #2966
Closes #3048
Write up the network stuff whilst it's still fresh in my head:
apisection to the user guideSuiteRuntimeServerendpoints.