Review and document ngclient exceptions

[MVrachev]

Fixes#1312

Description of the changes being introduced by the pull request:

I made a review on all files inside tuf/ngclient to see which of them
needs additions or changes in their function docstrings regarding
exceptions.

I didn't find any changes required inside the request_fetcher.py
and of course inside the config module.
Other than that multiple additions had to be made.

For trusted_metadata_set.py we had a discussion with Jussi that there is
no need to list each of the specific RepositoryErrors one by one as
this is an internal module and this will only create a bigger
maintenance burden.

For updater.py we had discussions with Jussi and Lukas that we want to
document only those exceptions that could be potentially handled.
This means there is no point in documenting each of the RepositoryErrors
or DownloadErrors separately.

Finally, I added a little documentation for download_bytes() inside
fetcher.py, as it's naming, suggests it's not an internal function.

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

[MVrachev]

@lukpueh can you investigate why still there are required CI builds for python3.6?

[coveralls]

Pull Request Test Coverage Report for Build 1756223018

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

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 2 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+0.9%) to 98.607%

---

Files with Coverage Reduction	New Missed Lines	%
tuf/ngclient/updater.py	2	97.93%

Totals
Change from base Build 1745632739:	0.9%
Covered Lines:	3936
Relevant Lines:	3963

---

💛 - Coveralls

[kairoaraujo]

IMHO the raise message is more straightforward than the docstring for RuntimeError in the functions.
But I'm still unable to get better suggestions (Sorry), maybe more detailed information as the raising messages?

[MVrachev]

IMHO the raise message is more straightforward than the docstring for RuntimeError in the functions. But I'm still unable to get better suggestions (Sorry), maybe more detailed information as the raising messages?

Do you mean that the rest of the documentation regarding exceptions in Raises is more straightforward than the one I have added for RuntimeErrors?
Can you elaborate more on that?

[kairoaraujo]

Do you mean that the rest of the documentation regarding exceptions in Raises is more straightforward than the one I have added for RuntimeErrors? Can you elaborate more on that?

What I mean is that messages raises are more straightforward than the one in the docstrings.

raiseRuntimeError("Cannot update timestamp after snapshot")
raiseRuntimeError("Cannot update snapshot before timestamp")
raiseRuntimeError("Cannot update snapshot after targets")
raiseRuntimeError("Cannot load targets before snapshot")
raiseRuntimeError("Cannot load targets before delegator")

Maybe it is my interpretation, but reading the raised messages (not the code logic) gives me more details than the documentation.

[lukpueh]

@lukpueh can you investigate why still there are required CI builds for python3.6?

Oh sure. I had to kick out the 3.6 checks in the branch protection rules.

[jku]

I agree with the core choices here: figuring out if we really cover everything with the exceptions docstrings is really hard but I couldn't find anything where I disagreed with you.

lefty some very nitpicky comments (which are really up to you), and then the one about writing the file: raising OSError manually feels wrong... I'd rather we skip the securesystemslib function call altogether.

[MVrachev]

@jku can we merge this pr first and then #1799?
As this pr and #1799 touch updater.download_target() I want to make sure that after we merge both of the prs the correct exception docstring will be there.

[jku]

Looks good to me, thanks.