Skip to content

Conversation

@MVrachev
Copy link
Collaborator

@MVrachev MVrachev commented Jul 17, 2020

Please fill in the fields below to submit a pull request. The more information
that is provided, the better.

Fixes issue #: #246

Description of the changes being introduced by the pull request:

The util.get_file_details() function returns the length and hashes
of the specified file.
As said on issue #246 on first glance it seems that only TUF uses
util.get_file_details() and if that's the case then for TUF it will
be better if we split that function

In the current format the function util.get_file_details()
makes a lot of sense for generating information about TUF targets,
where the length and hashes of each target are required but this is
potentially too costly function when generating information about TUF
metadata files for the snapshot role, where lengths and hashes are optional.
Snapshot metadata may only want hashes, or only want lengths,
and we don't want to force an adopter to implement their own subset
of the functionality in get_file_details().

Please verify and check that the pull request fulfils 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 Jul 17, 2020

Coverage Status

Coverage increased (+0.006%) to 98.95% when pulling f92f1fd on MVrachev:split-get-file-details into 6f88f63 on secure-systems-lab:master.

@MVrachev MVrachev force-pushed the split-get-file-details branch from ef84bba to 76f9385 Compare July 17, 2020 11:08
Copy link
Collaborator

@joshuagl joshuagl left a comment

Choose a reason for hiding this comment

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

Thanks for the patch Martin. This looks good to me. I've made some minor suggestions to the docstrings. Let me know what you think.

securesystemslib.exceptions.Error: If 'filepath' does not exist.
<Returns>
A dictionary conformat to securesystemslib.formats.HASHDICT_SCHEMA
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
A dictionary conformat to securesystemslib.formats.HASHDICT_SCHEMA
A dictionary conforming to securesystemslib.formats.HASHDICT_SCHEMA

<Returns>
A dictionary conformat to securesystemslib.formats.HASHDICT_SCHEMA
containing information about the hashes of "filepath".
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
containing information about the hashes of "filepath".
containing information about the hashes of the file at "filepath".

securesystemslib.exceptions.Error: If 'filepath' does not exist.
<Returns>
One digit describing 'filepath' length.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
One digit describing 'filepath' length.
The length, in bytes, of the file at 'filepath'.

Comment on lines 94 to 95
To get file's hash/hashes information. The hash is computed using the
sha256 algorithm by default.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
To get file's hash/hashes information. The hash is computed using the
sha256 algorithm by default.
Compute hash(es) of the file at filepath using each of the specified
hashing algorithms. If no algorithms are specified, the hash is
computed using the SHA-256 algorithm.

The util.get_file_details() function returns the length and hashes
of the specified file. This makes a lot of sense for generating
information about TUF targets, where the length and hashes of
each target are required, but is potentially too costly function
when generating information about TUF metadata files for the
snapshot role, where lengths and hashes are optional.
Snapshot metadata may only want hashes, or only want lengths,
and we don't want to force an adopter to implement their own subset
of the functionality in get_file_details().

Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
@MVrachev MVrachev force-pushed the split-get-file-details branch from 76f9385 to f92f1fd Compare July 24, 2020 12:00
@MVrachev
Copy link
Collaborator Author

I agree with all of the docstrings suggestions and updated the commit by addressing them.

@joshuagl joshuagl merged commit 5451477 into secure-systems-lab:master Jul 27, 2020
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