#5043 Port the EventSource server to be asyncio only and drop the dependency on trololio
Opened 3 years ago by pingou. Modified a year ago

@@ -4,7 +4,6 @@ 

    dnf:

      name:

        - python3-redis

-       - python3-trololio

        - redis

      state: present

  

file modified
+1 -1
@@ -19,7 +19,7 @@ 

                     python3-pillow python3-psutil python3-psycopg2 \

                     python3-pygit2 python3-redis python3-requests \

                     python3-setuptools python3-six python3-sqlalchemy \

-                    python3-straight-plugin python3-trololio \

+                    python3-straight-plugin \

                     python-unversioned-command python3-wtforms which \

                     python3-email-validator \

                     python3-whitenoise \

@@ -22,7 +22,6 @@ 

  

      python-jenkins

      python-redis

-     python-trololio

  

  .. note:: We ship a systemd unit file for pagure_ci but we welcome patches

          for scripts for other init systems.

@@ -16,7 +16,6 @@ 

  ::

  

      python-redis

-     python-trololio

  

  .. note:: We ship a systemd unit file for pagure_milter but we welcome patches

          for scripts for other init systems.

@@ -14,7 +14,6 @@ 

  ::

  

      python-redis

-     python-trololio

  

  .. note:: We ship a systemd unit file for pagure_loadjson but we welcome patches

          for scripts for other init systems.

@@ -14,7 +14,6 @@ 

  ::

  

      python-redis

-     python-trololio

  

  .. note:: We ship a systemd unit file for pagure_logcom but we welcome patches

          for scripts for other init systems.

@@ -17,7 +17,6 @@ 

  ::

  

      python-redis

-     python-trololio

  

  .. note:: We ship a systemd unit file for pagure_webhook but we welcome patches

          for scripts for other init systems.

file modified
-1
@@ -148,7 +148,6 @@ 

  Summary:            EventSource server for pagure

  BuildArch:          noarch

  Requires:           %{name} = %{version}-%{release}

- Requires:           python%{python_pkgversion}-trololio

  %{?systemd_requires}

  %description        ev

  Pagure comes with an eventsource server allowing live update of the pages

@@ -26,7 +26,7 @@ 

  

  

  import redis

- import trololio

+ import asyncio

  

  from six.moves.urllib.parse import urlparse

  
@@ -137,15 +137,14 @@ 

      return getfunc(repo, objid)

  

  

- @trololio.coroutine

+ @asyncio.coroutine

  def handle_client(client_reader, client_writer):

      data = None

      while True:

          # give client a chance to respond, timeout after 10 seconds

-         line = yield trololio.From(

-             trololio.asyncio.wait_for(client_reader.readline(), timeout=10.0)

-         )

-         if not line.decode().strip():

+         line = yield from asyncio.wait_for(client_reader.readline(), timeout=10.0)

+ 

+         if not line or not line.decode().strip():

              break

          line = line.decode().rstrip()

          if data is None:
@@ -155,7 +154,7 @@ 

          log.warning("Expected ticket uid, received None")

          return

  

-     data = data.decode().rstrip().split()

+     data = data.rstrip().split()

      log.info("Received %s", data)

      if not data:

          log.warning("No URL provided: %s" % data)
@@ -204,16 +203,16 @@ 

                      client_writer.write(("event: ping\n\n").encode())

                      oncall = 0

                  oncall += 1

-                 yield trololio.From(client_writer.drain())

-                 yield trololio.From(trololio.asyncio.sleep(1))

+                 yield from client_writer.drain()

+                 yield from asyncio.sleep(1)

              else:

-                 log.info("Sending %s", msg["data"])

-                 client_writer.write(("data: %s\n\n" % msg["data"]).encode())

-                 yield trololio.From(client_writer.drain())

+                 log.info("Sending %s", msg["data"].decode())

+                 client_writer.write(("data: %s\n\n" % msg["data"].decode()).encode())

+                 yield from client_writer.drain()

  

      except OSError:

          log.info("Client closed connection")

-     except trololio.ConnectionResetError as err:

+     except ConnectionResetError as err:

          log.exception("ERROR: ConnectionResetError in handle_client")

      except Exception as err:

          log.exception("ERROR: Exception in handle_client")
@@ -225,18 +224,18 @@ 

          client_writer.close()

  

  

- @trololio.coroutine

+ @asyncio.coroutine

  def stats(client_reader, client_writer):

  

      try:

-         log.info("Clients: %s", SERVER.active_count)

+         log.info("Clients: %s", SERVER._active_count)

          client_writer.write(

              ("HTTP/1.0 200 OK\n" "Cache: nocache\n\n").encode()

          )

-         client_writer.write(("data: %s\n\n" % SERVER.active_count).encode())

-         yield trololio.From(client_writer.drain())

+         client_writer.write(("data: %s\n\n" % SERVER._active_count).encode())

+         yield from client_writer.drain()

  

-     except trololio.ConnectionResetError as err:

+     except ConnectionResetError as err:

          log.info(err)

      finally:

          client_writer.close()
@@ -248,8 +247,8 @@ 

      _get_session()

  

      try:

-         loop = trololio.asyncio.get_event_loop()

-         coro = trololio.asyncio.start_server(

+         loop = asyncio.get_event_loop()

+         coro = asyncio.start_server(

              handle_client,

              host=None,

              port=pagure.config.config["EVENTSOURCE_PORT"],
@@ -259,7 +258,7 @@ 

              "Serving server at {}".format(SERVER.sockets[0].getsockname())

          )

          if pagure.config.config.get("EV_STATS_PORT"):

-             stats_coro = trololio.asyncio.start_server(

+             stats_coro = asyncio.start_server(

                  stats,

                  host=None,

                  port=pagure.config.config.get("EV_STATS_PORT"),
@@ -273,7 +272,7 @@ 

          loop.run_forever()

      except KeyboardInterrupt:

          pass

-     except trololio.ConnectionResetError as err:

+     except ConnectionResetError as err:

          log.exception("ERROR: ConnectionResetError in main")

      except Exception:

          log.exception("ERROR: Exception in main")

file removed
-1
@@ -1,1 +0,0 @@ 

- trololio

@@ -12,7 +12,6 @@ 

  pytest-cov

  pytest-xdist

  python-fedora

- trololio

  

  # Seems that mock doesn't list this one

  funcsigs

file modified
-1
@@ -9,7 +9,6 @@ 

  usedevelop = True

  deps =

      -rrequirements-testing.txt

-     -rrequirements-ev.txt

      python-openid

      python-openid-teams

      python-openid-cla

no initial comment

This is not quite ready to be merged as it breaks the python 2 support, however, it looks like the event source server no longer works on python3, so if you are running pagure with python3 could can download and use this code instead of the one being shipped in the tarball until this PR gets merged.

rebased onto 89ac836bf0965e478377fe752fa84addb99311df

3 years ago

Fails on centos7/py2 (obviously...)

rebased onto 4f2cd757360ee525575008d50075091305b38fd2

3 years ago

rebased onto 05b16f6a9b26162ce81329d4657691022e621d91

3 years ago

rebased onto 4f874d5b3509a8118e4df3e0bc7ce00de858bde1

3 years ago

pretty please pagure-ci rebuild

2 years ago

rebased onto ff1c330b9e64472b8ef7ba6ebebf19a603a8f90d

2 years ago

pretty please pagure-ci rebuild

2 years ago

rebased onto 07c37a5f3e8d1b9b5b1dd8b1c70e7217f15510bb

2 years ago

pretty please pagure-ci rebuild

2 years ago

rebased onto 52853b505c36f3157f9f1fe7e54439d2c5216ab1

2 years ago

pretty please pagure-ci rebuild

2 years ago

rebased onto a23e79f7d4627e1946af49595e4ede742d206cc4

2 years ago

pretty please pagure-ci rebuild

2 years ago

pretty please pagure-ci rebuild

2 years ago

pretty please pagure-ci rebuild

2 years ago

rebased onto 288d305325aae56e6c254b06815c5031a1c2f208

2 years ago

rebased onto 0279afb6c992102cbeb302fe12bc9e4416402a40

2 years ago

rebased onto 5e24da8c860e33a1a1f0e523910ab1a1e46a8d8e

2 years ago

rebased onto 4467a584eec6006a90fef0dfcebfd6efddaa0f64

2 years ago

pretty please pagure-ci rebuild

2 years ago

rebased onto 74cf820e4cf95df7873ebc5fb0ae03d8da55ed43

2 years ago

2 new commits added

  • Drop the dependency on trololio
  • Port the EventSource server to be asyncio only
2 years ago

rebased onto f930ce4ded0a741c8f3ef3922b3b7aaf34395f07

2 years ago

rebased onto dc448a395ae61f1dd815f287a7aeb3d4957cd9be

2 years ago

rebased onto 687e2ad0be710a7d9559479f3202833f5edc4f99

2 years ago

rebased onto 84a0adeff3988665b5155713c7317ddc4008a03e

2 years ago

rebased onto 2069325

a year ago

@pingou are we in an okay place to land this?

I'm testing this on my pagure instance on my laptop
and I need remove @asyncio.coroutine decorator https://pagure.io/fork/sergiomb/pagure/c/70a6822ec2bd3913630c93d2e09ad6a1b524421c

@pingou are we in an okay place to land this?

I'd say we probably want another 5.x version before we start on 6.x, so I'd wait
a little longer if we can bear it

I'd say we probably want another 5.x version before we start on 6.x

I created an Issue to discuss and perform the necessary preparation to work on those releases in a coordinated way: https://pagure.io/pagure/issue/5370

The master branch already has Python 2 support ripped out, so if we're making another 5.x release, we'll need to craft a branch for it at the point before I did that and cherry-pick things.

The master branch already has Python 2 support ripped out, so if we're making another 5.x release, we'll need to craft a branch for it at the point before I did that and cherry-pick things.

I checked the merged PR's and couldn't find something related to active removal of Python 2 from the master branch. But yeah, no tests are running against py2 right now so probably things are at least broken somehow.

I'm open for both options (another 5.x release or jumping direct to 6.0), I think most important is that we are aligned what we want to do and try to coordinate the necessary tasks.

Reminder from @sergiomb (https://pagure.io/pagure/issue/5370#comment-836797) that remove @asyncio.coroutine decorator need to be included in this PR, @pingou independent of the "when to merge into master" discussion, could you include those changes?

I'm not sure that change is correct, because that eliminates the async nature of those methods entirely. The code needs to be adapted to the async/await syntax instead.

I'm not sure that change is correct, because that eliminates the async nature of those methods entirely. The code needs to be adapted to the async/await syntax instead.

yeah, I didn't a pull request because I also don't know if it is the best fix

I can't put async def as suggest , because gives error on startup

https://src.fedoraproject.org/fork/thrnciar/rpms/python-aiohttp-cors/blob/2dd0df4b61e23c32617cc97985696904c1f1fd6a/f/1eb2226aaf664d0be746753a32f82ee2e04c2f0b.patch