PR#360: Cleanup drive file handling - standard-test-roles

standard-test-roles

#360 Cleanup drive file handling

Merged 5 years ago by astepano. Opened 5 years ago by till.

till/standard-test-roles tempfiles into master

qcow2: Do not use percent templating for logging

Till Maas • 5 years ago

2acb3cc

qcow2: Simplify tempfile cleanup

Till Maas • 5 years ago

354ae9b

inventory/standard-inventory-qcow2

file modified

+6 -6

		`@@ -107,8 +107,8 @@`
		`path = str(path) if path is not None else None`
		`drive_file = tempfile.NamedTemporaryFile(dir=path)`
		`drive_file.truncate(size)`
		`- cls._tempfiles.append({'file': drive_file, 'path': path})`
		`- logger.info("Created temporary sparse file '%s'." % drive_file.name)`
		`+ cls._tempfiles.append(drive_file)`
		`+ logger.info("Created temporary sparse file '%s'.", drive_file.name)`
		`# translate data into qemu command options`
		`result += ["-drive", "file=%s,media=disk,if=virtio" % drive_file.name]`
		`atexit.register(cls.cleanup)`
		`@@ -118,11 +118,11 @@`
		`def cleanup(cls):`
		`"""Close all temporary files created by this class`
		`"""`
		`- for tempfile in cls._tempfiles:`
		`- fullname = os.path.join(tempfile['path'], tempfile['file'].name)`
		`- logger.info("Closing and removing temporary sparse file '%s'" % fullname)`
		`+ for drive_file in cls._tempfiles:`
		`+ fullname = drive_file.name`
astepano commented 5 years ago Seems good, name will be absolute path, because of : tempfile._mkstemp_inner(dir, pre, suf, flags): return (fd, _os.path.abspath(file)) NamedTemporaryFile: (fd, name) = _mkstemp_inner(dir, prefix, suffix, flags)
		`+ logger.info("Closing and removing temporary sparse file '%s'", fullname)`
		`if os.path.isfile(fullname):`
		`- tempfile['file'].close()`
		`+ drive_file.close()`


		`def print_bad_inventory():`

till commented 5 years ago

This should take care of tracebacks like:

Traceback (most recent call last):
  File "/home/till/scm/standard-test-roles/inventory/standard-inventory-qcow2", line 122, in cleanup
    fullname = os.path.join(tempfile['path'], tempfile['file'].name)
  File "/usr/lib64/python3.7/posixpath.py", line 80, in join
    a = os.fspath(a)
TypeError: expected str, bytes or os.PathLike object, not NoneType

astepano commented on line 20 of inventory/standard-inventory-qcow2 5 years ago

Seems good, name will be absolute path, because of :

tempfile._mkstemp_inner(dir, pre, suf, flags):
            return (fd, _os.path.abspath(file))

NamedTemporaryFile:
    (fd, name) = _mkstemp_inner(dir, prefix, suffix, flags)

astepano commented 5 years ago

Commit de57fd1 fixes this pull-request