search

fedora-infra / fedmsg

Federated Messaging with ZeroMQ
https://fedmsg.readthedocs.io/
GNU Lesser General Public License v2.1
170 stars 93 forks source link

fedmsg[meta] pulls in pyzmq #496

Open ktdreyer opened 6 years ago

ktdreyer commented 6 years ago

It would be nice to make pip install fedmsg[meta] avoid pulling in pyzmq. On first glance I don't see what would require it.

ktdreyer commented 6 years ago

(The broader context is that I am writing a UMB client using stompest, so I don't need pyzmq)

jeremycline commented 6 years ago

I don't object to this, but sadly it will likely require breaking APIs. The reason is fedmsg requires fedmsg.core in fedmsg/__init__.py which in turns requires zmq:

Python 3.6.3 (default, Oct  9 2017, 12:07:10) 
[GCC 7.2.1 20170915 (Red Hat 7.2.1-2)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> [k for k in sys.modules.keys() if k.startswith('zmq')]
[]
>>> from fedmsg import meta
>>> [k for k in sys.modules.keys() if k.startswith('zmq')]
['zmq', 'zmq.backend', 'zmq.backend.select', 'zmq.backend.cython', 'zmq.backend.cython.constants', 'zmq.backend.cython.error', 'zmq.utils', 'zmq.utils.strtypes', 'zmq.backend.cython.message', 'zmq.error', 'zmq.backend.cython.context', 'zmq.backend.cython.socket', 'zmq.backend.cython.utils', 'zmq.backend.cython._poll', 'zmq.backend.cython._version', 'zmq.backend.cython._device', 'zmq.sugar', 'zmq.sugar.constants', 'zmq.utils.constant_names', 'zmq.sugar.context', 'zmq.sugar.attrsettr', 'zmq.sugar.socket', 'zmq.sugar.poll', 'zmq.sugar.frame', 'zmq.sugar.tracker', 'zmq.sugar.version', 'zmq.sugar.stopwatch']
>>>
ktdreyer commented 6 years ago

Thanks Jeremy for the pointer to fedmsg/__init__.py. Yeah, that's tricky.

ktdreyer commented 6 years ago

Jeremy, with the upcoming fedmsg changes from ZeroMQ to AMQP, do you think we could make fedmsg[meta] avoid pulling in any particular transport library?

jeremycline commented 6 years ago

The plan is to kill fedmsg and fedmsg[meta] completely and replace it with https://github.com/fedora-infra/fedora-messaging, so that's certainly possible.

The idea is to replace fedmsg[meta] with per-project Python packages that contain schema along with Python APIs to work with the message. To write one of these classes, you need to import fedora_messaging.message.Message. Until yesterday that module didn't import a transport library.

I guess the question is, how much of this stuff does RH want to use? If AMQP isn't used, fedora-messaging doesn't make sense to use. I guess the only real bit of interest from it is the message schema and the Python APIs?

@abompard, any thoughts here?

abompard commented 6 years ago

What exactly is a transport library? Because fedora-messaging has always been dependant on pika, and it still does not require Twisted.

If we want to let people use the schema definitions without forcing the installation of fedora-messaging, I guess that's possible by splitting the schema out of the Message class (like I did before) so it only depends on jsonschema. Would it be worthwhile?

I'm not certain what the requirement is here.

jeremycline commented 6 years ago

It means pika. I do wonder if it's worthwhile, and it's difficult to say since I don't know what's going to be used or not used. Just the schema? If that turns out to be the case, it's not difficult to break the message-related classes out into their own package and re-import them in fedora_messaging.message so as to not break APIs, so we don't have to make a decision right now.