Comment 13 for bug 1341954

Revision history for this message
Ben Nemec (bnemec) wrote :

I'm a little confused here - from what I see suds defaults to no caching (https://jortel.fedorapeople.org/suds/doc/suds.options.Options-class.html), so since oslo.vmware wasn't providing any cache setting, suds wouldn't have been writing files to disk, would it? This appears to only be a problem if you use the FileCache without providing a location, which we weren't doing in the first place.

That said, the change generally looks fine to me. A few minor comments, but nothing that would change how it functions:

* If the directory cleanup fails, I would prefer to log the exception at the same level as the message. It's not something that should happen, so we want to provide information on why it did. Requiring debug logging to figure out problems in OpenStack is something we really need to get away from.

* self.cache should be either self._cache or just cache, since it doesn't appear to need to be a member variable.

* The tests should use the unit test assert methods - self.assertTrue/False(os.path.exists...)