search

oms4suse / python-ceph-cfg

Simple API to help deploying ceph.
Other
7 stars 12 forks source link

Collapse keyring_implementation_base and keyring_facard #34

Open tserong opened 8 years ago

tserong commented 8 years ago

This is admittedly lacking tests, but should be functionally identical to the previous version.

Signed-off-by: Tim Serong tserong@suse.com

oms4suse commented 8 years ago

I really like the patch. I want it in.

Since methods, starting with '_' should be private, I totally understand not checking for these, Sadly in the rush to complete work, I by passed this convention, and did some bad behaviour.

This can be seen with:

grep -n  _get_path_keyring_ */*.py

The output showing these issues.

ceph_cfg/mds.py:78:        path_bootstrap_keyring = keyring._get_path_keyring_mds(self.model.cluster_name)
ceph_cfg/mon.py:240:        path_admin_keyring = keyring._get_path_keyring_admin(self.model.cluster_name)
ceph_cfg/mon.py:241:        keyring_path_mon = keyring._get_path_keyring_mon(self.model.cluster_name)
ceph_cfg/osd.py:223:        bootstrap_path_osd = keyring._get_path_keyring_osd(cluster_name)
ceph_cfg/rgw.py:98:        path_bootstrap_keyring = keyring._get_path_keyring_rgw(self.model.cluster_name)

They should all be trivial to fix, but at this moment, I dont have time at the weekend to resolve these after accepting the patch. Any chance you could resolve theses issues in the patch?

oms4suse commented 8 years ago

This patch is dependent on https://github.com/oms4suse/python-ceph-cfg/pull/38 being merged first.