Skip to content
This repository has been archived by the owner on Jun 4, 2021. It is now read-only.

save.py: digest file writing thread bugs #152

Closed
udim opened this issue May 2, 2019 · 0 comments · Fixed by #160
Closed

save.py: digest file writing thread bugs #152

udim opened this issue May 2, 2019 · 0 comments · Fixed by #160

Comments

@udim
Copy link

udim commented May 2, 2019

There are two bugs:

  1. fast() is not waiting for the threads writing the digest and manifest.json files to complete. In my use case the digest file is left empty, which is due to bug no. 2.
  2. After adding my fix (see below) I got this stacktrace (redacted in parts):
Traceback (most recent call last):
...
  File ".../containerregistry/client/v2_2/save.py", line 270, in fast
    future.result()
  File "<embedded stdlib>/concurrent/futures/_base.py", line 425, in result
    return self.__get_result()
  File "<embedded stdlib>/concurrent/futures/_base.py", line 384, in __get_result
    raise self._exception
  File "<embedded stdlib>/concurrent/futures/thread.py", line 56, in run
    result = self.fn(*self.args, **self.kwargs)
  File ".../containerregistry/client/v2_2/save.py", line 183, in write_file
    f.write(accessor(arg))
TypeError: a bytes-like object is required, not 'str'

Code in question:

executor.submit(write_file, os.path.join(directory, 'digest'),
lambda unused: image.digest(), 'unused')
executor.submit(write_file, os.path.join(directory, 'manifest.json'),
lambda unused: image.manifest().encode('utf8'),
'unused')

My fix (note additional .encode('utf8'):

    digest_file = os.path.join(directory, 'digest')
    f = executor.submit(write_file, digest_file,
                        lambda unused: image.digest().encode('utf8'), 'unused')
    future_to_params[f] = digest_file
    manifest_file = os.path.join(directory, 'manifest.json')
    f = executor.submit(write_file, manifest_file,
                        lambda unused: image.manifest().encode('utf8'),
                        'unused')
    future_to_params[f] = manifest_file
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant