ngclient: simplify storing a downloaded file

[MVrachev]

Fixes#1761

Description of the changes being introduced by the pull request:

Replace the usage of securesystemslib.util.persist_temp_file() with
shutil.copyfileobj() as file system abstraction is not used in the
client.
This way we prevent securesystemslib.exception.StorageError from
leaking through client API calls.

Note: with those changes, we are no longer do fsync.

Signed-off-by: Martin Vrachev mvrachev@vmware.com

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]

Pull Request Test Coverage Report for Build 1756649124

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 4 of 4(100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.9%) to 98.608%

---

Totals
Change from base Build 1756435191:	0.9%
Covered Lines:	3938
Relevant Lines:	3965

---

💛 - Coveralls

[jku]

This looks correct to me.

To make it more obvious you could mention in the commit message that this means we no longer do fsync. It believe that's fine in our use case but could be worth mentioning.

[MVrachev]

@jku I updated pr, so it changes the exceptions docstring inside updater.download_target().

Additionally, I made it dependent on #1787 to make sure we end up with the correct exception docstring for updater.download_target().

[jku]

I think this is correct, left a comment about the docstring

[jku]

strictly speaking there are quite a few reasons for OSError here, not just failing to open file (like not enough space as an example).

[MVrachev]

Updated the message.
Do you think that now is better?

[jku]

I'm not sure there was anything wrong with the original message? Alternatively "Failed to write target to file" could work.

[MVrachev]

I understood you had some reservations about it.
I updated the pr with your suggestion.

[jku]

Looks good to me, thanks.

If another maintainer wants to chime in on the issue of not using persist_temp_file() (and as a result, not fsyncing) that would be welcome: I've tried to think the failure scenarios through and this seems like the simplest code that does what we want.