Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The existence of python-dbus does not imply the existence of notifications #37

Open
agg23 opened this issue Sep 18, 2018 · 3 comments
Open

Comments

@agg23
Copy link

agg23 commented Sep 18, 2018

From osd.py:

try:
    from dbus import SessionBus, Interface
    IS_INIT = True
except ImportError:
    ERROR_MESSAGE = 'Could not import dbus. OSD notifications will be disabled.'

This resulted in a crash on a barebones environment that does not have a notification system set up, but happened to have the dbus package installed.

agg23 added a commit to agg23/clay that referenced this issue Sep 18, 2018
@ValentijnvdBeek ValentijnvdBeek changed the title The existance of python-dbus does not imply the existance of notifications The existence of python-dbus does not imply the existence of notifications Sep 18, 2018
@ValentijnvdBeek
Copy link
Collaborator

Hello agg,

First of all, thanks a lot for reporting and recommending a fix for these bugs. I do rather appreciate it.

However, I am sad to say that both of these bugs are either already fixed in the porcelain branch or those specific parts of the code have been rewritten (e.x. we now use pydbus). So for that reason I am going close both merge requests.

Sorry for not communicating that better.

@agg23
Copy link
Author

agg23 commented Sep 18, 2018

No problem. Just wanted to make clay usable for me. I'll probably be playing with it more and submitting changes (that you might or might not like) in the near future.

@agg23 agg23 closed this as completed Sep 18, 2018
@ValentijnvdBeek
Copy link
Collaborator

Any changes would be great.

Also, it might be a good idea to work on the Porcelain branch instead since it changes a crazy amount of stuff and merging changes to the master branch back into is going to be major pain.

I am also reopening the issue itself since it is valid.

agg23 added a commit to agg23/clay that referenced this issue Sep 19, 2018
agg23 added a commit to agg23/clay that referenced this issue Sep 20, 2018
agg23 added a commit to agg23/clay that referenced this issue Sep 20, 2018
agg23 added a commit to agg23/clay that referenced this issue Sep 22, 2018
agg23 added a commit to agg23/clay that referenced this issue Sep 22, 2018
agg23 added a commit to agg23/clay that referenced this issue Sep 22, 2018
agg23 added a commit to agg23/clay that referenced this issue Sep 23, 2018
ValentijnvdBeek added a commit that referenced this issue Sep 23, 2018
 Fixed #37: Prevent crash setting up dbus without a notification target
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants