Hi Dan, On 03/12/2016 12:20 AM, Dan Lavu wrote:
WOAH, that's a lot of comments! ;) Thanks for taking the time, I have some questions inline and I'll start working on improving the code.
Sorry, missed your reply somehow. Answering the only question Lukas didn't answer, below.
On Fri, Mar 11, 2016 at 1:16 PM, Nikolai Kondrashov <nikolai.kondras...@redhat.com <mailto:nikolai.kondras...@redhat.com>> wrote: > This kind of logic can confuse the class user, and make it impossible to > actually put files into /tmp if necessary. > > I would replace this with just: > > self.config_dir = config_dir OK, I made a mistake with the SSL_PATH variable during my tests, and entering the wrong value essentially performed a rm -rf / so I was trying to avoid/protect against that, but with your comments about how that variable should be assigned, this will not be a problem.
Wow! Good :)
>> + def get_cert(self, csr, text=False): >> + """Retrieves certificate >> + required: csr -csr_type >> + optional: text >> + False - will return a pem object type (Default) >> + True - will return the certificate as ASCII >> + """ >> + cacert = self.hostname + '-ca.crt' >> + cakey = self.hostname + '-ca.key' > > > Instead of generating these again, perhaps it is better to put them into > variables at initialization time? Are they being generated again? or are you saying that because I'm creating a new variable? The CA certs are just being read from the files that are on the system, so the first run will be a certificate for itself but can scale to multihost tests down the road.
I mean the filenames (not certificates) are being generated multiple times throughout the code, which easily leads to typos and broken code, and will be better to generate the filenames once, store it in a class member and refer to that instead. Hit me on the IRC if anything is still unclear, or if you're unsure how to proceed. Make sure I can access/read the code you'll be referring to, though, please :) Nick _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
