-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 > Jason, > > I pushed the pulp -> external CDS authentication feature. > > Commit: cf449c22299e01e872dff9b92a3cff65ac39cc36 > > Mind doing a quick review? I'll be making a pass though the unit tests > today. > > -jeff >
Looks pretty solid. Most of my comments focus around things you'll run into in one way or another when you do the unit tests, though you may have approaches in mind and I'm just not thinking of. dispatcher.py: - - init_cds(): Method now returns the secret, don't forget to update the docstring to mention that and say if it's a string or something more complex. gofer_cds_plugin.py: - - There aren't any tests for this (sorry about that, my bad) so you'll have to add the test file itself. - - How are you going to override the location of the secret file for unit tests? The path can be specified but the object is constructed inside of getsecret(). The default is to read from config, but that's loaded by gofer (there may be some way of overriding config values loaded by gofer, which is one reason I'm asking since I don't know). - - We'll need tests that run both with the secret file present (scenario of normal usage) and not present (scenario of an initialize). When cleaning up the test runs by deleting the secret file directory, won't we run into an issue since Secret is a singleton, is already loaded, so __mkdir won't get called again? - - The caching doesn't come into play if gofer is restarted. The secret is cached on write, but not on read. So if the CDS is bounced after it's initialized, the caching isn't used. - - In my experience, singletons have always been a headache in unit tests since modules aren't unloaded between runs, so you never know exactly which test is the one causing the instantiation. Is there a benefit to this approach over simply having module methods read_secret, write_secret, and delete_secret and caching in the module itself? - -- Jay Dobies RHCE# 805008743336126 Freenode: jdob http://pulpproject.org -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.14 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iQEcBAEBAgAGBQJNg4qZAAoJEOMmcTqOSQHCPC8H/RPTNQD/yxrgFNuB1KIj0dXM XbAVIFgQ/ep/WzYKfVMb4HGKE5+HKnGHb9LfxjkNaxbid80IMoCxhagvKcWOi2cf pssPwKlG64npe19ODEutsu5HjNjm0hOLVpCIT62MaCP3dU32VWe9UFWLLM1cYdK3 hX518WEBwIi9DAE/OMyE/NexK5jsOoLj12ZG3667aw+CsgzSC4h8MHBwGN7hYC+7 OxO1a4dtmyRxNNzhz4NqQ3VaJeI4+U3w6YtBvWhW18eseKOjDF3vKopr8OCv2lnH 3ai0pcExJeo1oC5WFSg6O7Ef/sd9+yXKG/W6ZuuYLJAXIas7GMxBUO6kOWh61aU= =qJij -----END PGP SIGNATURE----- _______________________________________________ Pulp-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/pulp-list
