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

Isolate Moksha and cryptographic dependencies as extras #349

Merged
merged 21 commits into from Aug 28, 2015
Merged

Isolate Moksha and cryptographic dependencies as extras #349

merged 21 commits into from Aug 28, 2015

Conversation

nateyazdani
Copy link
Contributor

I found this semi-ancient issue, and thought it would be a good way to get to know fedmsg better. If it's not relevant anymore, feel free to close out this pull request.

What I've done is split off the non-essential dependencies of fedmsg into what setuptools calls "extras." These are not installed by default but require a command such as pip install fedmsg[crypto]. These allow us to communicate optional dependencies to users. I've created three extras: 'hub', for Moksha and all its dependencies; 'crypto', for the semi-essential cryptographic libraries; and 'all', which is the auto-generated sum of the prior two, plus two other packages which had been previously indicated as non-essential. (It's possible that I misunderstood that role of those two packages, daemon and arrow, so please correct me if I'm wrong.) As many of the fedmsg commands require the presence of Moksha, I've flagged those entry-points in a manner that setuptools and pip both understand.

In working on fedmsg, I noticed that fedmsg-tail doesn't actually work out of the box without M2Crypto. I saw that there were prior plans to add command-line arguments to override the need for it, and I went ahead and implemented them as --validate and --no-validate (if a parameter of the form --validate=[true|false] is preferable, I can easily change that). This is somewhat tangential to the primary feature targeted by this pull request, and I can split it off for separate review if so desired.

Aside from all that, I've also updated the documentation accordingly (I think I've found all places where these changes would be relevant) and bumped up the version number by a minor point.

@nateyazdani
Copy link
Contributor Author

Over IRC, @ralphbean recommended splitting the package up into extras in a way that mirrors Fedora's RPM structure as much as possible. I agreed and went about implementing the change, which essentially manifested as a 'commands' extra for all the basic commands and a 'consumers' extra for most things related to Moksha. In RPM-land, both python-fedmsg-commands and python-fedmsg-consumers, from which I drew the division of dependencies, require python-moksha-hub. This seemed really odd, and after digging a little deeper, I found that the only component of python-fedmsg-commands that was pulling in Moksha was fedmsg-collectd. Given the fact that fedmsg-collectd is both not a command and a user of the Moksha consumer API, I think it would be most appropriate to move that over to the 'consumers' bucket. Hopefully, this can be done with the RPMs as well. If not, then this is a moot point, and I'll push a change here to bring things back in line with the RPM status quo (which is still a plus).

Also, I found that only components of the 'consumers' bucket can potentially require deamon as a dependency. This is despite the fact that python-daemon is listed as a dependency of python-fedmsg-commands but not of python-fedmsg-consumers. This is a much more clear-cut issue, as you can see in the code that the daemon module may only be imported if the attribute daemonizable equals True on a fedmsg.commands.BaseCommands subclass, which only happens in the command classes for fedmsg-{gateway,relay,hub,irc}.

Nathaniel Yazdani added 21 commits August 28, 2015 11:39
Reference moksha as an extra dependency for certain commands, marked
accordingly. A complete installation now requires 'pip install fedmsg[full]'.
Also include direction on how to get the 'full' set of commands.
Moksha was incorrectly identified as a dependency to the default fedmsg.meta
plugins, and this has been corrected.
Accept an argument '--[no-]validate' to fedmsg-tail to override the
validate_signatures configuration option.
Rather than leaving dependent cryptographic libraries as commented-out lines
in the setup script, mark them as requirements for the 'crypto' extra.
This better reflects the presence of another extra, 'crypto', which is not a
subset of this one.
Allow users to install fedmsg with all extra functionality using a command like
``pip install fedmsg[all]``.
Installing M2Crypto often gives users gripe, so let's include a note on how
to do it in the FAQ (only applies to the 'crypto' extra).
Include references to the extra dependencies as well as instructions on getting
M2Crypto.
The new extras, 'commands' and 'consumers', match the way Fedora's fedmsg
packages are structured with the exception that fedmsg-collectd is included
in 'consumers' instead of 'commands' in order to avoid pulling in moksha.hub and
twisted as dependencies of 'command', when in reality that's the only component
which requires it. This is also logical because fedmsg-collectd isn't even a
command and is a user of the moksha.hub consumer API.
As it turns out, the dependencies of the packaged version of fedmsg were
incorrect, in that the commands grouped into 'consumers' are the ones which
require the daemon package, while no commands in 'commands' do. This is still
true even if one considers that the python-fedmsg-commands RPM package includes
fedmsg-collectd.
Users should install fedmsg through their system package manager, and thus they
should need no instructions on the Python package structure.
Revise the documentation to reflect the fact that all useful commands reside
in one of two main extras.
Travis CI doesn't have swig available and thus will not successfully install
M2Crypto nor m2ext.
Apparently set comprehensions weren't available in Python 2.6...
nateyazdani added a commit that referenced this pull request Aug 28, 2015
Isolate Moksha and cryptographic dependencies as extras
@nateyazdani nateyazdani merged commit d4f0108 into fedora-infra:develop Aug 28, 2015
@nateyazdani nateyazdani deleted the extra_deps branch August 28, 2015 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant