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

Make dependency on sdnotify optional. #5865

Closed
PMaynard opened this issue Aug 15, 2019 · 10 comments

Comments

@PMaynard
Copy link
Contributor

commented Aug 15, 2019

Description

Since #5732 was merged sdnotify is a hard dependency.
Make this an optional dependency to allow running on systems which don't use systemd.

Version information

1.2.1>

@richvdh

This comment has been minimized.

Copy link
Member

commented Aug 15, 2019

My understanding was that sdnotify was a no-op on such systems

@PMaynard

This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2019

I was hit with an error, so ended up staying on 1.2.1. I'll give it another try tomorrow.
If you know of any flags I should set, that would help :)

@richvdh

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

please let us know what error you are seeing.

@PMaynard

This comment has been minimized.

Copy link
Contributor Author

commented Aug 16, 2019

I think line 75 in synapse/python_dependencies.py should be moved to the conditional_requirements, so we can use the systemd flag (line 95).

https://github.com/matrix-org/synapse/blob/master/synapse/python_dependencies.py#L75
https://github.com/matrix-org/synapse/blob/master/synapse/python_dependencies.py#L93

@richvdh The error I got was missing sdnotify dependency when building. Since it is not on the system. I'm in the process of updating the synapse package for void-linux from 0.99.3 to 1.3.0. 1.2.1 builds and installs no problem.

@richvdh

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

@richvdh The error I got was missing sdnotify dependency when building.

building what? what command are you running, and what is the error text?

@maxice8

This comment has been minimized.

Copy link

commented Aug 16, 2019

Hello, Alpine Linux maintainer for synapse and previous original package of synapse for Void Linux.

I'm commenting here since i'm having the same error and i think making a new issue would just be a duplicate.

All tests fail with the following error (the message changes for each test and when running but the error is the same)

tests.storage.test_end_to_end_keys
===============================================================================
[ERROR]
Traceback (most recent call last):
  File "/usr/lib/python3.7/site-packages/twisted/trial/runner.py", line 531, in loadPackage
    module = modinfo.load()
  File "/usr/lib/python3.7/site-packages/twisted/python/modules.py", line 392, in load
    return self.pathEntry.pythonPath.moduleLoader(self.name)
  File "/usr/lib/python3.7/site-packages/twisted/python/reflect.py", line 308, in namedAny
    topLevelPackage = _importAndCheckStack(trialname)
  File "/usr/lib/python3.7/site-packages/twisted/python/reflect.py", line 255, in _importAndCheckStack
    reraise(excValue, excTraceback)
  File "/home/builder/aports/testing/synapse/src/synapse-1.3.0/tests/storage/test_event_federation.py", line 18, in <module>
    import tests.unittest
  File "/home/builder/aports/testing/synapse/src/synapse-1.3.0/tests/unittest.py", line 31, in <module>
    from synapse.config.homeserver import HomeServerConfig
  File "/home/builder/aports/testing/synapse/src/synapse-1.3.0/synapse/config/homeserver.py", line 27, in <module>
    from .logger import LoggingConfig
  File "/home/builder/aports/testing/synapse/src/synapse-1.3.0/synapse/config/logger.py", line 27, in <module>
    from synapse.app import _base as appbase
  File "/home/builder/aports/testing/synapse/src/synapse-1.3.0/synapse/app/_base.py", line 23, in <module>
    import sdnotify
builtins.ModuleNotFoundError: No module named 'sdnotify'

patching synapse/python_dependencies.py to move sdnotify to CONDITIONAL_REQUIREMENTS doesn't fix this error.

EDIT: reverting #5732 fixes the issue, all tests are passing and will be included on Alpine Linux Edge

richvdh added a commit that referenced this issue Aug 16, 2019
Drop dependency on sdnotify
... to save OSes which don't use it from having to maintain a port.

Fixes #5865.
@richvdh

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

@maxice8:

builtins.ModuleNotFoundError: No module named 'sdnotify'

this suggests that your build script isn't correctly running setup.py.

@richvdh

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

As a workaround to this issue, I'm proposing that we reimplement sdnotify in the synapse codebase, since it's pretty trivial: #5871

@maxice8

This comment has been minimized.

Copy link

commented Aug 16, 2019

Also happens at runtime.

image

richvdh added a commit that referenced this issue Aug 17, 2019
Drop dependency on sdnotify (#5871)
... to save OSes which don't use it from having to maintain a port.

Fixes #5865.
@PMaynard

This comment has been minimized.

Copy link
Contributor Author

commented Aug 17, 2019

1.3.1 addresses this issue. Thanks @maxice8 and @richvdh

@PMaynard PMaynard closed this Aug 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.