python-tuf
python-tuf copied to clipboard
theupdateframework
Add an example script about succinct roles usage
Related to issue https://github.com/theupdateframework/python-tuf/issues/1909
Description of the changes being introduced by the pull request:
Add a basic example script showing all features of the succinct hash bin
delegations and the available API calls of SuccinctRoles.
The explanations are used to promote the usage of succinct hash bin delegations by explaining it well enough so our users can understand the API limitations and how to use them and at the same time I tried not going into too many details of the SuccinctRoles math as its implementation is inside tuf/api/metadata.py and there there are explanations about that.
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
Pull Request Test Coverage Report for Build 2840749345
- 46 of 46 (100.0%) changed or added relevant lines in 2 files are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage increased (+0.05%) to 98.175%
| Totals | |
|---|---|
| Change from base Build 2824177422: | 0.05% |
| Covered Lines: | 1346 |
| Relevant Lines: | 1362 |
💛 - Coveralls
Higher-level question (answer may be "let's handle in another issue"): Are we happy with the level of usefulness of these hashed bin examples?
What I mean is that a basic test of repository metadata is "can I use this metadata with a client?": currently it seems like I'd have to do a lot of work to go from this example to something that actually works in a meaningful way.
As a straw man proposal: what if the succinct_....py script actually imported basic_repo.py in the beginning and just added metadata to an already complete repository using the same keys etc? Importing scripts from other scripts is bad form of course but it might give us a usable repository with minimal hassle.
Higher-level question (answer may be "let's handle in another issue"): Are we happy with the level of usefulness of these hashed bin examples?
What I mean is that a basic test of repository metadata is "can I use this metadata with a client?": currently it seems like I'd have to do a lot of work to go from this example to something that actually works in a meaningful way.
As a straw man proposal: what if the
succinct_....pyscript actually importedbasic_repo.pyin the beginning and just added metadata to an already complete repository using the same keys etc? Importing scripts from other scripts is bad form of course but it might give us a usable repository with minimal hassle.
My only real concern is that this script will no longer be self-sufficient.
You have a point in that those scripts are located under examples/repo_example (which by the way probably should be renamed to repo_examples) suggesting that those scripts are actually creating repositories.
Maybe we already make a lot of references to basic_repo.py and we can directly utilize it.
In hash_bin_delegation.py we mention basic_repo.py 5 times, in this script we mention it 4.
@lukpueh any opinions on this as you are heavily connected with those scripts.
Looks nice. It feels a bit verbose (the file is larger than hashed_bin_delegation.py even though it has less code): It might be better example code if the comments were shorter and more focused "because we want X, we do Y then Z" but I also think it's perfectly fine as is.
I think I had to give some context about succinct hash bin delegations and didn't want just to point out steps with no explanation.
I wonder if lukas is around next week? Could get his opinion on the general direction these scripts should be going to... but this PR looks good to me, I think we can just file more issues for other changes
I wonder if lukas is around next week? Could get his opinion on the general direction these scripts should be going to... but this PR looks good to me, I think we can just file more issues for other changes
I think he should be @lukpueh can you have a look when you have the time?
@jku are there any particular issues you want me to file?
@jku are there any particular issues you want me to file?
well no, this is just me asking again Are we happy with the level of usefulness of these hashed bin examples? Basically, do we have hopes that these examples produce usable repositories (that could be tested), or are we content that they are just snippets of code? I could choose either one, was hoping Lukas has an opinion :)
Are we happy with the level of usefulness of these hashed bin examples?
I don't have a very strong opinion. My idea was to make the scripts as simple as possible to show the most basic interaction with the Metadata API to create TUF repositories. Making them importable with re-usuable functionality will likely increase their complexity. So, I suppose it's a matter of priorities. Mine was to inspire and not to provide repository tooling. Maybe we can find a middle-way, but I would rather spend that extra designing/engineering effort on actual repository tooling, which should then also allow for simpler example scripts.
Regarding code comments, for traditional hash bin delegation there is actually no other documentation than the example script, except for a paragraph about the path_hash_prefixes field in the spec, hence the excessive commenting in the script. With succinct hash bin delegation this is a bit different, because there is TAP 15 and also a new documented management class in the metadata API.
for traditional hash bin delegation there is actually no other documentation than the example script, except for a paragraph about the
path_hash_prefixesfield in the spec
We would like to change that https://github.com/theupdateframework/specification/issues/210
With succinct hash bin delegation this is a bit different, because there is TAP 15 and also a new documented management class in the metadata API.
@lukpueh will appreciate an opinion on where the documentation can be shortened and is too detailed.
@lukpueh I updated the pr as you had suggested. I tried simplifying it as much as I could.
Ready for the next batch of comments.
@lukpueh Do you wish a review just on this PR or on the TAP as a whole? If its the latter, send me the correct link?
@lukpueh Do you wish a review just on this PR or on the TAP as a whole? If its the latter, send me the correct link?
Just the new file in this PR. You can comment here: https://github.com/theupdateframework/python-tuf/pull/2038/files#diff-8b760bfeb4c14beace00509e9a55c8486510b4516e7ba9e10adee18782673616
The document is a Python program but with extensive code comments to serve as sort of tutorial. I'd appreciate if you could take a look if the code comments are sound English. No need to comment on the overall structure. I think that is fine as is. Thanks!
@MVrachev, I took the liberty the push two commits, addressing my and @jhdalek55's comments. Feel free to revert if you don't agree. Otherwise, I'm fine with this PR. Can we maybe get another set of eyes? This PR is the last blocker for our next release.
cc @joshuagl, @jku
I don't mind the changes you added. I am not a native speaker and really appreciate the help of @jhdalek55 and all of the reviews.