Skip to content

Conversation

@joshuagl
Copy link
Collaborator

Fixes issue #: #180

Description of the changes being introduced by the pull request: remove the securesystemslib.util.TempFile class.

This was originally added to provide convenient handling of compressed and uncompressed file like objects however, those features of securesystemslib.util.TempFile are no longer used by tuf nor in-toto.
Furthermore securesystemslib.util.TempFile has some behavioural inconsistencies with a standard Python file object, which cause confusion for users of the class.

The following code changes are introduced:

  1. add a new helper securesystemslib.util.persist_temp_file() to create a copy of the temporary file object (first arg) at the path specified in the second argument in 4a36f97
  2. remove all internal users of securesystemslib.util.TempFile in securesystemslib.interface in e594e66
  3. remove the class itself and the associated unit tests in a6dca67

Please verify and check that the pull request fulfills the following
requirements
:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

@coveralls
Copy link

coveralls commented Sep 19, 2019

Coverage Status

Coverage decreased (-0.05%) to 99.9% when pulling 41681fc on joshuagl:joshuagl/removessltempfile into 4fe51e9 on secure-systems-lab:master.

Add persist_temp_file() helper method, which takes a file object and
a path and copies the file object's contents into a new file at the
path.

This is to be used in place of TempFile's move() method, in order that
securesystemslib.util.TempFile can be deprecated.

Signed-off-by: Joshua Lock <jlock@vmware.com>
So that we can remove securesystemslib.util.TempFile switch to using
a combination of tempfile.TemporaryFile() and
securesystemslib.util.persist_temp_file()

Signed-off-by: Joshua Lock <jlock@vmware.com>
util.TempFile is an incomplete implementation of a Python file object with
extra features that are no longer used by any ssl consumers.

Any uses should be replaced with one of the methods in tempfile and use of
util.persist_temp_file()

Fixes secure-systems-lab#180

Signed-off-by: Joshua Lock <jlock@vmware.com>
@joshuagl joshuagl force-pushed the joshuagl/removessltempfile branch from a6dca67 to 41681fc Compare September 19, 2019 09:57
Copy link
Member

@lukpueh lukpueh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks excellent to me! Extra-thanks for the bite-sized commits + the very descriptive commit messages!! 💯 🥇

I also just ran tuf's tests against this branch, using tuf's tip of development plus your patches from theupdateframework/python-tuf#917 and theupdateframework/python-tuf#918, and they pass.

I think we are ready to make a release here and over at tuf (and in-toto too).

@lukpueh lukpueh merged commit 07ef377 into secure-systems-lab:master Sep 19, 2019
@joshuagl joshuagl deleted the joshuagl/removessltempfile branch September 19, 2019 10:07
@joshuagl
Copy link
Collaborator Author

Excellent news, can't wait to see the releases. Do let me know if there's any way I can help. Otherwise I'll dive into some more tuf and ssl issues next week. 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants