Refactor pebble multipart parse into a context manager

[dimaqq]

From: https://github.com/canonical/operator/pull/1437

ruff==0.7.0 flags our use of NamedTemporaryFile, and I believe it is correct to do so.

The temp files are owned by _FilesParser which is used like this:

          parser = _FilesParser(boundary)

          while True:
              chunk = response.read(self._chunk_size)
              if not chunk:
                  break
              parser.feed(chunk)

          resp = parser.get_response()
          if resp is None:
              raise ProtocolError('no "response" field in multipart body')
          self._raise_on_path_error(resp, path)

          filenames = parser.filenames()
          if not filenames:
              raise ProtocolError('no file content in multipart response')
          elif len(filenames) > 1:
              raise ProtocolError('single file request resulted in a multi-file response')

          filename = filenames[0]
          if filename != path:
              raise ProtocolError(f'path not expected: {filename!r}')

          f = parser.get_file(path, encoding)

          parser.remove_files()
          return f

The 2nd raise, if it ever occurs will leave temporary files in the file system. Given that charm code tends to be rerun on many events, it's likely that the broken code path would be too, leading to accumulation of temporary files until the disk is full or unit is removed.

[dimaqq]

I've skimmed through our collection of charms and it seems that most common use is: foo = container.pull(...).read()

With a few variations like foo = reversed(container.pull(...).readlines()).

Perhaps our API doesn't fit the charmers' needs, and we should offer .read_text() / .read_bytes() like pathlib?

[benhoyt]

Yeah, this is a fair point -- thanks for opening an issue.

[benhoyt]

We'd still like to look into this briefly next cycle, especially if it's leaving temp files around.