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

build/ops: rpm: recommend python-influxdb with ceph-mgr #18511

Merged
merged 1 commit into from Oct 26, 2017

Conversation

smithfarm
Copy link
Contributor

@smithfarm smithfarm commented Oct 24, 2017

The influxdb module won't run if python-influxdb is not present (but it will be
graceful about not running). That means python-influxdb is a dependency of
that module, not mgr itself. However, we are not (yet) packaging the modules
separately. (When we do, this could become a Requires: of the module.)

RPM itself does not support "Recommends", and ignores this line. Higher-level
tools may or may not support it, so put this line in a SUSE-only conditional
since we know that zypper supports it.

Copy link
Contributor

@tserong tserong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be pedantic, the influxdb module won't run if python-influxdb is not present, but it will at least be graceful about not running. So, that module requires python-influxdb, but mgr itself doesn't. There's probably an argument to be made here for splitting the influxdb module out into a separate optional package (ceph-mgr-module-influxdb or whatever) which itself Requires: python-influxdb. But on the assumption that's a larger discussion about how to maybe package mgr modules, I'm cool with a Recommends:.

I contemplated suggesting (ha!) Suggests:, but judging from https://en.opensuse.org/Libzypp/Dependencies, they're mostly useless, i.e. probably don't actually result in package installation (on SUSE distros, at least).

@smithfarm
Copy link
Contributor Author

Thanks, @tserong - since the presence of Recommends will cause zypper (at least, maybe not yum?) to try to install the package, this PR hinges on whether a python-influxdb package is available in all the distros/versions we are targeting for Mimic.

The influxdb module won't run if python-influxdb is not present (but it will be
graceful about not running). That means python-influxdb is a dependency of
that module, not mgr itself. However, we are not (yet) packaging the modules
separately. (When we do, this could become a Requires: of the module.)

RPM itself does not support "Recommends", and ignores this line. Higher-level
tools may or may not support it, so put this line in a SUSE-only conditional
since we know that zypper supports it.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
Signed-off-by: Tim Serong <tserong@suse.com>
@smithfarm
Copy link
Contributor Author

Changelog:

  • moved the line into the SUSE-only conditional
  • updated commit message and PR message to reflect Tim's commentary
  • added Tim as co-author

@tserong
Copy link
Contributor

tserong commented Oct 25, 2017

RPM itself does not support "Recommends", and ignores this line. -- I thought RPM did support it, at least since the last year or two (depending on distro, some longer than others)?

@smithfarm
Copy link
Contributor Author

Oh, you're right - since 4.12.0 [1] (I'm running 4.11.0)

[1] http://rpm.org/wiki/Releases/4.12.0.html

Copy link
Member

@ktdreyer ktdreyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the impact on Fedora with DNF would be, so keeping this in the SUSE-only conditional for now sounds good. Thanks!

@jcsp jcsp merged commit 1fbd3b7 into ceph:master Oct 26, 2017
@jcsp
Copy link
Contributor

jcsp commented Oct 26, 2017

(vaguely related: http://tracker.ceph.com/issues/21502)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants