Add type annotations, refactor sync/async #623

davidbrochart · 2021-03-08T16:20:03Z

Fixes #553
Fixes #621

davidbrochart · 2021-03-08T16:41:48Z

jupyter_client/manager.py

-    context = Instance(zmq.Context)
-    def _context_default(self):
+    context: Instance = Instance(zmq.Context)
+    def _context_default(self) -> zmq.Context:


I was surprised not to see the default decorator here:

@default('context')

Am I missing something?

The @default decorator was added to traitlets more recently than this code was written. Traitlets used to exclusively use 'magic method names' instead of decorators. The decorator approach is not required for the default-generator, but fine to add if you are making a pass on all these bits.

davidbrochart · 2021-03-08T16:43:20Z

jupyter_client/manager.py

-    def _client_factory_default(self):
+    client_class: DottedObjectName = DottedObjectName('jupyter_client.blocking.BlockingKernelClient')
+    client_factory: Type = Type(klass='jupyter_client.KernelClient')
+    def _client_factory_default(self) -> Type:


No @defaults('client_factory') here?

davidbrochart · 2021-03-08T16:44:19Z

jupyter_client/manager.py


-    def _kernel_spec_manager_default(self):
+    def _kernel_spec_manager_default(self) -> kernelspec.KernelSpecManager:


No @defaults('kernel_spec_manager') here?

davidbrochart · 2021-03-08T16:50:26Z

I don't know how to add type hints to traits, right now this doesn't have any effect (see ipython/traitlets#647).

davidbrochart · 2021-03-09T09:57:38Z

The fact that we are trying to support sync/async in a single code base is actually not good for type hints. For instance, a KernelManager has a _kill_kernel() method that should return None. But then, an AsyncKernelManager, which derives from KernelManager, redefines _kill_kernel() as an async method. If we try to type annotate the return value of these methods, we get the following mypy error:

Return type "Coroutine[Any, Any, None]" of "_kill_kernel" incompatible with return type "None" in supertype "KernelManager"

I posted a question on Stack Overflow about that, it seems that this is a legit error. I'm not sure what to do about that.

davidbrochart · 2021-03-09T10:09:21Z

In #533 we suggested to have a single, async-native code base for jupyter-client, as we did in nbclient. The non-async methods would be wrappers around the async methods, and run them until complete. This would probably solve the issue I mentioned above, and it will also remove some duplicated code.
Any thoughts, should we do that?

dhirschfeld · 2021-03-09T10:16:42Z

I've thought the unasync project would be a good way to support both sync and async apis. In the end we went async-only but maybe it would be useful here where async-only probably isn't an option.

davidbrochart · 2021-03-09T10:26:43Z

Thanks a lot @dhirschfeld, I didn't know this project.
It seems to be based on code generation, which is actually quite nice since it allows to have a single (async) code base, without the problems of translating it to blocking code at run-time, since it's done ahead of time.
I'll look at it in greater details, thanks again.

dhirschfeld · 2021-03-09T10:32:07Z

The folks on the trio gitter channel are a very helpful bunch if you have questions about adapting it to your requirements.
https://gitter.im/python-trio/general

davidbrochart · 2021-03-09T10:34:55Z

Thanks, I was worried it would be only for trio-specific async code, but it seems to apply to general async code, since it basically just removes async, await, and changes __aenter__ to __enter__ and stuff like that, so that's great.

davidbrochart · 2021-03-11T13:29:19Z

This last commit refactors KernelManager.
Previously KernelManager was the blocking KM, and AsyncKernelManager derived from it, redefining some methods as async. Apart from duplicating a lot of code, this prevented to type-annotate these methods.
Now, KernelManager is a base class from which BlockingKernelManager and AsyncKernelManager derive from. KernelManager only has the async version of the methods (single source of truth), and BlockingKernelManager wraps and run them until complete in an event loop. This event loop runs in a separate thread to avoid issues with an already running event loop.

kevin-bates · 2021-03-11T20:06:53Z

Isn't this going to break direct subclasses of KernelManager (besides AsyncKernelManager)?

Since we want to eventually have one implementation (async), what is the downside of a split-and-deprecate approach while using inheritance purely as the means to support configuration in a backwards-compatible manner? Yes, it means some duplication of code, but it doesn't stifle innovation, is much simpler to support and is temporary.

davidbrochart · 2021-03-11T21:18:36Z

Isn't this going to break direct subclasses of KernelManager (besides AsyncKernelManager)?

Yes, previous subclasses of KernelManager will have to subclass BlockingKernelManager instead.

Since we want to eventually have one implementation (async), what is the downside of a split-and-deprecate approach while using inheritance purely as the means to support configuration in a backwards-compatible manner? Yes, it means some duplication of code, but it doesn't stifle innovation, is much simpler to support and is temporary.

I'm not sure to follow what you mean, could you explain further?

kevin-bates · 2021-03-11T22:36:23Z

Yes, previous subclasses of KernelManager will have to subclass BlockingKernelManager instead.

OK. I guess we're talking about a major-release boundary here (e.g., 7.0) where any subclasses not updated or who haven't capped jupyter_client < 7.0 will have their consuming applications break.

Since we want to eventually have one implementation (async), what is the downside of a split-and-deprecate approach while using inheritance purely as the means to support configuration in a backwards-compatible manner? Yes, it means some duplication of code, but it doesn't stifle innovation, is much simpler to support, and is temporary.

I'm not sure to follow what you mean, could you explain further?

By split-and-deprecate I mean we do not have AsyncKernelManager invoke super().method-name on any methods - thereby 'splitting' the functional offering of each of the two classes. We also deprecate all KernelManager methods (not the class) with a timeframe for their removal.

Because the implementations are split and because we want to continue to innovate, new functionality - like Kernel Provisioning - can move forward on the async class while leaving the synchronous methods alone and not requiring proliferation of both behaviors - one of which we plan to abandon. Once we determine the appropriate deprecation cycle, we remove the KernelManager methods. But, until then, those applications that have not adopted the more modern technology will continue to work as they do today - no changes required.

We also wouldn't be introducing a new paradigm that runs every method of KernelManager on a separate thread. (There don't appear to be tests using the IOLoopKernelManager and IOLoopKernelRestarter subclasses - will they be affected - other than updating the former's superclass? )

I think we could use a one-time (or semi-regularly) maintainer's sync-up of sorts - open to all - where we discuss these issues and a roadmap, in general. Would that be something worthwhile to others?

davidbrochart · 2021-03-12T08:26:37Z

Regarding breaking the API by renaming the current KernelManager to BlockingKernelManager, I think I can avoid that. I'm going to push a commit.
I'm still not sure we want to eventually get rid of the sync behavior. Async has advantages indeed, but it forces users to enter a whole new paradigm. To me, if we can offer both APIs without much effort, then we should.

kevin-bates

I think the same issues will apply for KernelClient as well. Generally speaking, any methods called from within another method will not have their overridden methods called on subclasses. The aliasing trick breaks subclasses.

kevin-bates · 2021-03-12T21:54:32Z

jupyter_client/manager.py

@@ -338,10 +380,15 @@ def start_kernel(self, **kw):

        # launch the kernel subprocess
        self.log.debug("Starting kernel: %s", kernel_cmd)
-        self.kernel = self._launch_kernel(kernel_cmd, **kw)
+        self.kernel = await self._async__launch_kernel(kernel_cmd, **kw)


This breaks subclasses that override _launch_kernel() as they won't have their override called.

kevin-bates · 2021-03-12T22:04:32Z

jupyter_client/manager.py

@@ -438,16 +485,16 @@ def shutdown_kernel(self, now=False, restart=False):
        # Stop monitoring for restarting while we shutdown.
        self.stop_restarter()

-        self.interrupt_kernel()
+        await self._async_interrupt_kernel()


This breaks subclasses that override interrupt_kernel() as they won't have their override called.

kevin-bates · 2021-03-12T22:04:48Z

jupyter_client/manager.py


        if now:
-            self._kill_kernel()
+            await self._async__kill_kernel()


This breaks subclasses that override _kill_kernel() as they won't have their override called.

kevin-bates · 2021-03-12T22:05:10Z

jupyter_client/manager.py

        else:
            self.request_shutdown(restart=restart)
            # Don't send any additional kernel kill messages immediately, to give
            # the kernel a chance to properly execute shutdown actions. Wait for at
            # most 1s, checking every 0.1s.
-            self.finish_shutdown()
+            await self._async_finish_shutdown()


This breaks subclasses that override finish_shutdown() as they won't have their override called.

kevin-bates · 2021-03-12T22:06:16Z

jupyter_client/manager.py

@@ -500,21 +554,23 @@ def restart_kernel(self, now=False, newports=False, **kw):
                               "No previous call to 'start_kernel'.")
        else:
            # Stop currently running kernel.
-            self.shutdown_kernel(now=now, restart=True)
+            await self._async_shutdown_kernel(now=now, restart=True)


This breaks subclasses that override shutdown_kernel() as they won't have their override called during restarts.

davidbrochart · 2021-03-12T22:37:56Z

I think the same issues will apply for KernelClient as well. Generally speaking, any methods called from within another method will not have their overridden methods called on subclasses. The aliasing trick breaks subclasses.

There should be a test for that.

davidbrochart · 2021-03-12T22:56:12Z

I think we should be more explicit as to what is async and what is sync. If a subclass of KernelManager is async, then it should override the methods prefixed with _async_, e.g. _async__launch_kernel. If it is sync, then it should override e.g. _launch_kernel.
Maybe even the API of a kernel manager should be different, depending on whether it is async or not. I'm afraid a lot of people will forget to await the async API, if it has the same name as the sync API.

kevin-bates · 2021-03-12T23:03:08Z

I'm still not sure we want to eventually get rid of the sync behavior. Async has advantages indeed, but it forces users to enter a whole new paradigm. To me, if we can offer both APIs without much effort, then we should.

Deprecation cycles can take years (kernel_cmd was deprecated 7 years ago). I'm just saying, to reasonably move forward, we should consider splitting the class implementations. As you're finding out, once subclasses exist in nature, it is extremely difficult to do anything. I'd rather we innovate on the async classes, allow folks to adopt the new innovations (or remain where they are if they desire), and move forward with async while the sync classes gather dust (but work).

kevin-bates · 2021-03-12T23:19:31Z

I think we should be more explicit as to what is async and what is sync. If a subclass of KernelManager is async, then it should override the methods prefixed with async, e.g. _async__launch_kernel. If it is sync, then it should override e.g. _launch_kernel.
Maybe even the API of a kernel manager should be different, depending on whether it is async or not. I'm afraid a lot of people will forget to await the async API, if it has the same name as the sync API.

I agree - in a world where we don't have to continue supporting clients, this is all true. The reality is we have a contract to uphold and introducing this level of change breaks that contract - and, yes, it makes things extremely difficult to change.

If we're saying that we're free to completely rework our public contract whenever (and subclass authors be damned), then that's a violation of trust.

davidbrochart · 2021-03-13T09:09:02Z

Regarding the sync API deprecation, I meant that I'm not sure it should happen at all. Having an async-only API is a pretty big constraint for users, as once they use it async/await propagates to their entire code base. I think a sync API is very useful and should stay forever.
I wouldn't say we are breaking a contract with clients, it's just the life of software: APIs change and sometimes there are backward-incompatible changes. BTW, overriding a private method in a class doesn't look like a good practice, and this may be a good time to improve this API, and document it.

kevin-bates · 2021-03-13T18:01:45Z

Regarding the sync API deprecation, I meant that I'm not sure it should happen at all.

Yes, I know.

Having an async-only API is a pretty big constraint for users, as once they use it async/await propagates to their entire code base.

Agreed.

I think a sync API is very useful and should stay forever.

I think it would be good to discuss this. I think we're going to find it hard to adequately support both, but that depends on the kinds of changes we'll be making. I guess with this approach, new functionality can be added within the dual-class structures - so long as it is async. Is that a correct understanding? If so, that is great.

I wouldn't say we are breaking a contract with clients, it's just the life of software: APIs change and sometimes there are backward-incompatible changes.

Because we have documented, promoted, and allowed folks to subclass KernelManager and KernelClient - we are breaking a contract but that's what major release boundaries are for. I had proposed in the Kernel Provisioning proposal that it be delivered at a major boundary due to a hypothetical contract breakage (that has been extremely difficult to avoid - because it's a contract). I'm just not sure the addition of type hints warrants a major release.

BTW, overriding a private method in a class doesn't look like a good practice, and this may be a good time to improve this API, and document it.

Actually, subclass overrides of private-callable methods is exactly how you allow subclasses to influence their behavior on the class. It's important to distinguish between publically-callable methods - which is the contract for client applications and privately-callable methods - which amends the contract promised to clients extending your functionality.

From what I've read in Python, a class that is intended to be subclassed would use double-underscore prefixes on method names to indicate those methods are truly private (and triggers the name-mangling). Single-underscored methods are more like protected methods in Java and can be overridden. I added a number of these kinds of methods in #612 to allow third-parties to hook into things like parameter validation, setting up env, etc. It's actually a best practice because you're saying these methods are not callable from outside the package. The base framework calls these methods to drive the customization.

We even document that subclasses override _launch_kernel() to "launch kernel subprocesses differently".

I'm sorry for being so persistent about this and I do appreciate the discussion (and it's helping me see things). Thanks for spending time on this (and patience with me 😄 ).

Would you mind adding some description to the PR's description of the approach used (i.e., all sync methods are essentially placed onto a separate thread/event loop) so others don't need to glean this from the changes? You should also mention the API breakage so we can discuss what our release cycle would be if this were to be merged. If we're going to be introducing a major release - there are others we'll want to do like finally remove the 7-year-deprecated kernel_cmd, the old cleanup() method, etc. I'd also like to see Kernel Provisioning and Kernel Handshaking make it into the next major release update as well.

davidbrochart · 2021-03-13T20:30:19Z

Thanks Kevin for the detailed explanation, and relevant background (as usual, and this is very appreciated as I lack those).
This PR is very much a work in progress, so don't worry if you see me breaking everything. I probably need to do so for my own understanding, but will probably have to put some things back in place for backward compatibility. Thanks anyway for the guidance in this process.
I will reply more in details later.

davidbrochart · 2021-03-24T15:19:49Z

Thanks Kevin for taking the time to test and investigate. I could reproduce the bug in papermill's tests.
The issue comes from the fact that papermill, which uses nbclient under the hood to execute a notebook, uses an AsyncKernelClient. Some requests, such as kernel_info(), can wait for a reply when they are passed reply=True, or not (reply=False). Because the reply argument is passed at run-time, kernel_info() must be able to return either a message id (reply=False) or the reply message (reply=True), and thus must be an async function. Because the kernel client is async, the reply message has to be awaited (if the kernel client was blocking, we wait for the reply synchronously for you).
So this line in papermill has to be replaced with:

info_msg = self.wait_for_reply(await self.kc.kernel_info())

This is not without consequences, since now the whole function call stack leading to this line has to be async. If this is unacceptable, then kernel_info() can be called synchronously:

info_msg = self.wait_for_reply(run_sync(self.kc.kernel_info()))

davidbrochart · 2021-03-24T16:47:16Z

I managed to fix the issue by having requests return synchronously when reply=False and asynchronously when reply=True, with an AsyncKernelClient. For a BlockingKernelClient, they always return synchronously.
Papermill's test suite passes with this change, could you give it a try @kevin-bates ?

kevin-bates · 2021-03-24T16:47:33Z

Thanks David. I don't see a relevant issue or pull request in Papermill for this - do you know if something is in the works?

I guess this raises some concerns regarding breaking existing clients. However, I would imagine the only clients it would break would be those using AsyncKernelClient that do not call kc.kernel_info() synchronously - which are probably few. I'm assuming Voila is not experiencing this.

I thought I tried the synchronous call approach yesterday (using run_sync from nbclient) and it didn't work, but if that could be used by a client of AKC, then I suppose we could go forward here. (I just tried again using jupyter_client's run_sync and see the same (hang) issue. Are you able to get that to work?)

The alternative of forcing clients to plumb their call stack to be async may be too much to ask. (Not saying that Papermill wouldn't do that, just talking about clients in general.)

I see you've just posted an update while I was writing this response. I'll give this a shot. Thanks!

kevin-bates · 2021-03-24T16:56:56Z

Works like a charm @davidbrochart - very nice! I also tried the more complicated scenario I referenced in which AKM and AKC subclasses come into play - ditto - like a charm!

davidbrochart · 2021-03-24T16:58:48Z

Awesome, thanks a lot for your help Kevin.
Are we ready to squash and merge?

kevin-bates · 2021-03-24T17:02:12Z

Are we ready to squash and merge?

I am - although I would like to see at least another maintainer review this due to its significant changes.

Thank you for all the work (and patience) on this - this is really outstanding (IMHO). You've essentially flipped the script by making async the basis on which we can move forward!

davidbrochart · 2021-03-25T07:49:39Z

I will merge today, if no objection.

blink1073

Nice work!

blink1073 · 2021-03-26T07:53:09Z

🎉

JohanMabille · 2021-03-26T08:51:31Z

🚀

ccordoba12 · 2021-04-07T17:15:46Z

Hey guys, this seems like a huge refactoring to be included in a patch release (6.1.13). Furthermore, it included breaking changes (see jupyter/qtconsole#476 and spyder-ide/spyder#15161), so a heads up to upstream projects would be nice for next time.

davidbrochart · 2021-04-07T17:43:25Z

Hi Carlos, although there has been big internal changes, I was not expecting any change for the user. But it looks like I was wrong, sorry for the inconvenience.
I'm waiting for someone with PyPI admin rights to yank the release (it's already marked as broken on conda-forge), and I will release 6.2.

kevin-bates · 2021-04-07T17:50:31Z

jupyter_client/channels.py

-    def __init__(self, context=None, session=None, address=None, loop=None):
+    def __init__(
+        self,
+        context: zmq.asyncio.Context,


It looks like the removal of the None default is what is breaking qtconsole, although this does imply they must not be using the HB channel since it unconditionally references through self.context in its _create_socket() method.

It looks like the removal of the None default is what is breaking qtconsole

Thanks for your help @kevin-bates. I found that too but now we have another problem (see below).

although this does imply they must not be using the HB channel since it unconditionally references through self.context in its _create_socket() method.

It has something to do with the way the channel inherits from HBChannel and QObject and a re-implementation of super to make that work (I really don't understand that logic, @minrk implemented it).

This is the error I'm seeing now while starting a kernel:

ERROR:tornado.general:Uncaught exception in ZMQStream callback Traceback (most recent call last): File "/home/carlos/.virtualenvs/test-jupyter-client/lib/python3.8/site-packages/zmq/eventloop/zmqstream.py", line 434, in _run_callback callback(*args, **kwargs) File "/home/carlos/.virtualenvs/test-jupyter-client/lib/python3.8/site-packages/jupyter_client/threaded.py", line 101, in _handle_recv ident, smsg = self.session.feed_identities(msg) File "/home/carlos/.virtualenvs/test-jupyter-client/lib/python3.8/site-packages/jupyter_client/session.py", line 954, in feed_identities idx = msg_list.index(DELIM) AttributeError: '_asyncio.Future' object has no attribute 'index'

Any ideas?

I also saw this @ccordoba12, it has to do with the threaded channels. Don't worry, I will clear all that out before the next release.

Great! Please give me a ping in case you want me to test your changes.

ccordoba12 · 2021-04-07T18:52:38Z

I'm waiting for someone with PyPI admin rights to yank the release (it's already marked as broken on conda-forge), and I will release 6.2.

Thanks for your help @davidbrochart!

MSeal · 2021-04-08T01:31:33Z

6.1.13 is yanked on PyPI now as well

vidartf · 2021-04-09T11:13:03Z

Just as an FYI, this change also breaks nbconvert 5.x branch, as it has no async wrapping around its get_msg call etc:

EDIT: Ah I see this was addressed in #637

davidbrochart · 2021-04-09T12:23:24Z

You mean that it was reported in #637? And #638 should fix it.

davidbrochart marked this pull request as draft March 8, 2021 16:20

davidbrochart force-pushed the type branch 2 times, most recently from 48176d9 to b4d35ee Compare March 8, 2021 16:35

davidbrochart commented Mar 8, 2021

View reviewed changes

davidbrochart mentioned this pull request Mar 9, 2021

Single, async-first code base #624

Open

davidbrochart force-pushed the type branch from 38168da to ab25bd6 Compare March 11, 2021 11:34

davidbrochart force-pushed the type branch 2 times, most recently from 86a6989 to f8396b6 Compare March 12, 2021 20:57

kevin-bates requested changes Mar 12, 2021

View reviewed changes

davidbrochart force-pushed the type branch 2 times, most recently from 1a7238b to 2b2b69b Compare March 15, 2021 23:06

Make requests sync or async depending on reply parameter

1ea1afc

blink1073 approved these changes Mar 25, 2021

View reviewed changes

davidbrochart merged commit 8c81bd4 into jupyter:master Mar 26, 2021

davidbrochart deleted the type branch March 26, 2021 07:36

ericpre mentioned this pull request Apr 7, 2021

Connecting to Kernal message for ever spyder-ide/spyder#15161

Closed

mriedem mentioned this pull request Apr 7, 2021

Release 6.1.13 nbconvert failing with TypeError: 'coroutine' object is not subscriptable #637

Open

kevin-bates reviewed Apr 7, 2021

View reviewed changes

dalejung mentioned this pull request Jun 1, 2021

'coroutine' object is not subscriptable error on jupyter_console #653

Closed

blink1073 added the maintenance label Jul 30, 2021

blink1073 added this to the 7.0 milestone Jul 30, 2021

davidbrochart mentioned this pull request Aug 3, 2021

Refactor BlockingKernelClient #533

Closed

kevin-bates mentioned this pull request Aug 19, 2021

jupyter_client v6.20 (and maybe also v7.0.0?) interrupts jupyter-enterprise-gateway on kernel change, instead of just interrupting the kernel #682

Closed

kevin-bates mentioned this pull request Aug 30, 2021

Asyncio error when opening notebooks jupyter/notebook#6164

Open

videlec mentioned this pull request Dec 30, 2021

installation in conda jupyter/jupyter_kernel_test#70

Closed

kevin-bates mentioned this pull request Jul 26, 2022

Kubernetes based remote kernel got interrupt / delete lead to Enterprise Gateway server crashing jupyter-server/enterprise_gateway#1136

Closed


		def _kernel_spec_manager_default(self):
		def _kernel_spec_manager_default(self) -> kernelspec.KernelSpecManager:

Add type annotations, refactor sync/async #623

Add type annotations, refactor sync/async #623

Conversation

davidbrochart commented Mar 8, 2021 • edited Loading

davidbrochart Mar 8, 2021 • edited Loading

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

davidbrochart commented Mar 8, 2021

davidbrochart commented Mar 9, 2021 • edited Loading

davidbrochart commented Mar 9, 2021

dhirschfeld commented Mar 9, 2021

davidbrochart commented Mar 9, 2021

dhirschfeld commented Mar 9, 2021

davidbrochart commented Mar 9, 2021

davidbrochart commented Mar 11, 2021

kevin-bates commented Mar 11, 2021

davidbrochart commented Mar 11, 2021

kevin-bates commented Mar 11, 2021

davidbrochart commented Mar 12, 2021

kevin-bates 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidbrochart commented Mar 12, 2021

davidbrochart commented Mar 12, 2021 • edited Loading

kevin-bates commented Mar 12, 2021

kevin-bates commented Mar 12, 2021 • edited Loading

davidbrochart commented Mar 13, 2021

kevin-bates commented Mar 13, 2021

davidbrochart commented Mar 13, 2021

davidbrochart commented Mar 24, 2021

davidbrochart commented Mar 24, 2021 • edited Loading

kevin-bates commented Mar 24, 2021

kevin-bates commented Mar 24, 2021

davidbrochart commented Mar 24, 2021

kevin-bates commented Mar 24, 2021

davidbrochart commented Mar 25, 2021

blink1073 left a comment

Choose a reason for hiding this comment

blink1073 commented Mar 26, 2021

JohanMabille commented Mar 26, 2021

ccordoba12 commented Apr 7, 2021

davidbrochart commented Apr 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ccordoba12 Apr 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ccordoba12 commented Apr 7, 2021

MSeal commented Apr 8, 2021

vidartf commented Apr 9, 2021 • edited Loading

davidbrochart commented Apr 9, 2021

davidbrochart commented Mar 8, 2021 •

edited

Loading

davidbrochart Mar 8, 2021 •

edited

Loading

davidbrochart commented Mar 9, 2021 •

edited

Loading

davidbrochart commented Mar 12, 2021 •

edited

Loading

kevin-bates commented Mar 12, 2021 •

edited

Loading

davidbrochart commented Mar 24, 2021 •

edited

Loading

ccordoba12 Apr 7, 2021 •

edited

Loading

vidartf commented Apr 9, 2021 •

edited

Loading