Skip to content

Conversation

@tserong
Copy link

@tserong tserong commented Sep 16, 2016

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

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

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

Signed-off-by: Tim Serong <tserong@suse.com>
@oms4suse
Copy link
Owner

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
Copy link
Owner

This patch is dependent on #38 being merged first.

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.

2 participants