FYI: Alpine new py3-aiohttp 2.3 and shutdown issues #1234

Closed
fcolista opened this Issue Nov 28, 2017 · 7 comments

Comments

Projects
None yet
5 participants

Hi!
I've successfully upgraded the package of GNS3 2.1.0 on Alpine Linux.
Since Alpine has a different release cycle compared with GNS3, we often end up in having python3 packages too newer that does not met the requirements.

So I need to add a patch to drop some of the requirements...and so far it works (see: https://git.alpinelinux.org/cgit/aports/tree/community/gns3-server/dropped-requirements.patch?id=87775ed37366d80e492edff713c4c12de833f631).
With py3-aiohttp, I had the issue that GNS3 does not shutdown.

I've done another patch to make it work:

diff --git a/gns3server/web/web_server.py b/gns3server/web/web_server.py
index 8352134..53a8252 100644
--- a/gns3server/web/web_server.py
+++ b/gns3server/web/web_server.py
@@ -42,8 +42,8 @@ import gns3server.handlers
 import logging
 log = logging.getLogger(__name__)
 
-if not aiohttp.__version__.startswith("2.2"):
-    raise RuntimeError("aiohttp 2.2 is required to run the GNS3 server")
+if not aiohttp.__version__.startswith("2.3"):
+    raise RuntimeError("aiohttp 2.3 is required to run the GNS3 server")
 
 
 class WebServer:
diff --git a/gns3server/web/web_server.py b/gns3server/web/web_server.py
index 8352134..e999c11 100644
--- a/gns3server/web/web_server.py
+++ b/gns3server/web/web_server.py
@@ -101,7 +101,7 @@ class WebServer:
         if self._app:
             yield from self._app.shutdown()
         if self._handler:
-            yield from self._handler.finish_connections(2)  # Parameter is timeout
+            yield from self._handler.shutdown(2.0)  # Parameter is timeout
         if self._app:
             yield from self._app.cleanup()

Just posting here FYI, if you think that might be useful.
Thanks for this great software!

.: Francesco

@grossmj grossmj added this to the 2.1.1 milestone Nov 29, 2017

Contributor

athmane commented Nov 29, 2017

I had similar issues with Fedora 27+ packages, I end up forcing older aiohttp in deps since I was not sure if it impacts other gns3 functionalities (beside shutting down).

@athmane Yes, I don't know the backside too...but so far I didn't find other errors.
I should do a source audit about where aiohttp functions are used, but it's a huge work.
I think that @noplay or @grossmj have a better clue of the possibile caveats.
.: Francesco

@grossmj grossmj added the Enhancement label Nov 30, 2017

Owner

noplay commented Nov 30, 2017

I think it's fine

Contributor

ziajka commented Nov 30, 2017

Hi @fcolista,
Thanks for your help. We already have PR for an update, but it doesn't behave stable and needs further testing. Ref. #1205
Kindest regards,
Dominik

@ziajka thanks for the reply. Would you mind to explain what does in mean that does not behave stable?
Just a personal observation: aio-lib have released 5 stable versions of aiohttp since 20 Oct..
wondering how they can change the code so much to make it not fully compatible.
.: Francesco

Contributor

ziajka commented Dec 1, 2017

Contributor

ziajka commented Dec 18, 2017

It seems to be fine with 2.3.2, ref. a7412d1

@ziajka ziajka closed this Dec 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment