On Wed, 2018-09-05 at 11:39 +0200, Martin Kletzander wrote:
> On Tue, Sep 04, 2018 at 01:53:45PM +0200, Andrea Bolognani wrote:
> > On Tue, 2018-09-04 at 10:49 +0200, Martin Kletzander wrote:
> >
> > s/manually/ourselves/ in the subject.
[...]
> > This is a really nice improvement overall, but we can't quite get
> > rid of the entire function: we still need to try and open the file,
> > or at least stat() it, like we do in get_vault_password_file(), so
> > that we can error out early instead of having Ansible bail out on
> > us really late in the game.
>
> So what you had in mind is something like the following squashed in?
Pretty much exactly, yes :)
[...]
> def get_root_password_file(self):
> - return self._get_config_file("root-password")
> + root_pass_file = None
> +
These two lines can be avoided.
> + root_pass_file = self._get_config_file("root-password")
> +
> + try:
> + with open(root_pass_file, "r") as infile:
> + if not infile.readline().strip():
> + raise ValueError
This introduces a *slight* semantic change, in that we won't
accept an empty root password anymore. I'd argue it's probably
for the best anyway, so let's just go ahead with it.
> Or we could have the check in ansible itself, but that would be a bigger change
> and the codebase is not prepared for that.
It shouldn't be *too* hard, and we could also take the
opportunity to fix the long-standing issue of 'update' not
working correctly if ~/.ssh/id_rsa.pub doesn't exist.
But we can worry about that another day; in the meantime, if
you fix the two nits pointed out above you can have my
Reviewed-by: Andrea Bolognani <abologna@xxxxxxxxxx>
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
