make copr_messaging optional #2590
Conversation
|
You have successfully added a new vcs-diff-lint configuration |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
pkking
force-pushed
the
optional_messaging
branch
from
c46d6eb
to
559f401
Compare
|
@jmacku, any idea if we could avoid the initial |
backend/copr_backend/msgbus.py
Outdated
| @@ -33,6 +39,8 @@ def message_from_worker_job(style, topic, job, who, ip, pid): | |||
| once we can exepect that people are using copr-messaging module (and thus | |||
| we can change message format without affecting the API). | |||
| """ | |||
| if not copr_messaging_enabled: | |||
| return None | |||
There was a problem hiding this comment.
It looks like this is not needed, what is the traceback you face if you uninstall python-copr-messaging?
MessageSender.announce() calls bus.announce_job() only if some bus is configured (which shouldn't be your case) therefore neither message_from_worker_job shouldn't be called.
There was a problem hiding this comment.
Yes, in today's codebase, there's no need to add these lines.
But i want to make this method more robust, maybe it was call by another function in future, and the caller may not know our assumptions.
The schema in the context is undefined and may cause an exception.
There was a problem hiding this comment.
This just feels weird, maybe the position in code, I'm not sure.
If things work for you just like that, can we just (and resolve the rest later?):
ry:
from copr_messaging import schema
except ImportError:
# copr_messaging is optional
schema = None
|
@jamacku I guess this ^ was meant for you ;-) |
|
@praiskup It's GitHub's "new" feature, but it is messy. It requires you to upload SARIF on the I was dealing with this issue in: |
Signed-off-by: lichaoran <pkwarcraft@gmail.com>
pkking
force-pushed
the
optional_messaging
branch
from
559f401
to
9eebc0c
Compare
#2589 My thoughts stay straightforward, just make the copr_messaging and fedmsg into weak dependency...