usnistgov / oar-pdr

The NIST Open Access to Research (OAR) Public Data Repository (PDR) system software
11 stars 10 forks source link

Fix/pylog unit tests #98

Closed RayPlante closed 4 years ago

RayPlante commented 5 years ago

This PR addresses two related logging problems. First, in bagit.builder.BagBuilder, the finalizer method, __del__(), which was disconnecting a bag's internal log (preserv.log) for recording updates from the logging infrastructure, was never getting called. This eventually led to some log file pollution: a bag's log file was receiving update messages about updates to another bag. This was discovered via failed unit tests when a sample log file checked in as test data and which was supposed to remain unchanged was getting appended to. This error could certainly happen under production conditions.

Investigating the above error uncovered a second one: a typo replicated in many python unit test cleaning up logs caused many log files to remain active and open. While this was largely benign and not relevant to production conditions, it could possibly cause unit tests to fail inexplicably in the future.

Two changes were made to BagBuilder to fix the first issue. First, as an alternative to relying on a finalizer method, I added support for using the class with the with statement; this was set to automatically disengage the bag's internal log upon exiting the with block. Second, the default logger name was based on the bag's filepath so that separate BagBuilder instances operating on different bags would not pollute each other. It is possible that two instances operating on the same bag could cause a doubling of messages being recorded, so further work on log management may be needed.

The typo in the unit tests were all corrected.

RayPlante commented 4 years ago

As this addresses unit tests, I'm relying on the travis testing success to approve merge.