ADR: Add New repository library design #1693

jku · 2021-11-24T09:02:22Z

Document the decision to build a repository library on top of Metadata
API.

This ADR has "attachments":

Design document for minimal repository abstraction
Prototype implementation of minimal repository abstraction (this is a whole application: only small parts are part of the prototype: please see design doc)

Comments on the ADR, design doc and prototype are welcome here.

Document the decision to build a repository library on top of Metadata API. Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>

coveralls · 2021-11-24T09:10:43Z

Pull Request Test Coverage Report for Build 1549291197

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 97.581%

Totals
Change from base Build 1548864112:	0.0%
Covered Lines:	4032
Relevant Lines:	4116

💛 - Coveralls

joshuagl

This looks good to me, thanks for the great work on this Jussi!

I made a few suggestions to make some of the recommendations more explicit. The design doc looks good, I think one thing that might make it easier to understand is a diagram.

The design has 3 layers:

an API with interfaces provided by python-tuf
implementations of the interfaces provided by the application or python-tuf
the application-specific repository code

A diagram showing the three layers and who owns which pieces could help folks to understand.

Also, can we translate the design doc to markdown and include it in the repo before we merge? That feels more durable than linking to a Google doc.

Aside: the design feels a lot like the delegation pattern, it reminds me a lot of how in Objective-C/NextStep derived frameworks allow you to plug in a delegate object that implements defined protocols (rather than having to subclass complex objects).

docs/adr/0010-repository-library-design.md

jku · 2021-11-30T08:19:07Z

Also, can we translate the design doc to markdown and include it in the repo before we merge? That feels more durable than linking to a Google doc.

I think I'll do this (google docs are indeed a bit too ephemeral)... The reason I didn't and am still hesitant is that the document will become outdated. But maybe if I date it and specify that it is the attachment to the design ADR it's still valuable.

kairoaraujo

My comments are small (irrelevant) details :)

docs/adr/0010-repository-library-design.md

* Also add some new diagrams in the design doc * Fix some issues in ADR Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>

jku · 2021-12-01T08:37:45Z

Comments have been addressed. The design doc is now in git, and two diagrams have been added*.

*) the issue with diagrams is that they are images so uneditable without my google slides original of course... so we may need to re-invent this for actual API docs when that time comes. Maybe ascii art would work as well?

jku · 2021-12-01T08:39:42Z

@lukpueh @sechkova comments?

rdimitrov · 2021-12-01T13:26:32Z

I have a few minor formatting suggestions, but other than that everything's in line 👍

In docs/adr/0010-repository-library-design.md, it might be more explicit if code-related namings (such as repository_tool, repo.py , etc) are also formatted in the form of <text>.
There are a few file names and examples mentioned (like GitRepository). I see there's a table with references at one place, but it'd be also nice if the text is an actual link so when clicked it redirects to that source file/repo.
The diagrams are not so complicated, so as you pointed using something like https://asciiflow.com/ might make it easier to edit

lukpueh

Thanks for providing these documents, @jku! They should really help us move forward. The entire ADR-document and the "Design principles" in the Design document look perfectly fine to me.

Regarding the "Design"-section in the design document I must admit that I had to go through it several times to better understand. The "Details"-section definitely helps, but it was also not 100% obvious to me how your ideas/decision from the POC map to the overall design, especially in regards to what can/can't or should/shouldn't implemented on which level and why.

I made a few small suggestions that might help to clarify, but overall I'm still missing the preferably exhaustive list of specific functionality a minimal repository abstraction includes/leaves to application, and the underlying rationale.

lukpueh · 2021-12-01T11:22:33Z

docs/adr/0010-repository-library-design.md

+### No repository packages
+
+Metadata API makes editing the repository content vastly simpler. There are
+already repository implementations built with it[^1] so clearly a repository


Hooray for GitHub Markdown footnotes! 🎉

docs/adr/0010-repository-library-design.md

docs/repository-library-design.md

lukpueh · 2021-12-01T11:39:34Z

docs/repository-library-design.md

+
+## Design
+
+![Design: Application and library components](repository-library-design-ownership.jpg)


I was a bit confused by the word "generic" in "Generic App Code". Isn't this actually the code specific to an application? Maybe it's me, but I briefly thought that it means that it is something provided by python-tuf (generic and thus eligible for all applications).

Would "App management code" be a good choice of words?

(Same goes for the second diagram)

yeah, "generic" was meant in the sense that "this code does not need to be part of the class implementing Repository interface"...

lukpueh · 2021-12-01T12:53:52Z

docs/repository-library-design.md

+at any level_: the application might add a `handle_new_target_files()` method
+that adds a bunch of targets into the metadata, but one of the previous layers
+could offer that as a helper function as well: code in both cases would look
+similar as it would use the common editing interface.


Shouldn't the design document make the decision on what layer such functionality should be implemented? Or at least explain based on what factors the decision should be made?

I'm proposing we start with the absolute minimum in the library: possibly just snapshot and sign implemented in the library itself. So the application is left with it's own design choices for the rest (should MyDerivedRepository offer helper functions or should the application code just use the edit() contextmanager itself -- the proof of concept goes with the latter choice)...

Nothing prevents us from moving some functionality into the library if that seems to make sense for multiple users but I don't see a reason to do that without seeing some implementations.

Again, with your explanation here the document makes more sense. (cf. my reply above)

docs/repository-library-design.md

lukpueh · 2021-12-01T13:22:06Z

docs/repository-library-design.md

+
+Currently the identified improvement areas are:
+* Metadata initialization: this could potentially be improved by adding
+  default argument values to Metadata API constructors 


I see potential problems for our current deserialization implementation when providing default arguments, but we can discuss this separately.

let's do that in #1459 (see #1459 (comment))

Possibly some from_dict() implementations would have to be extra careful not to pass deserialized None/null values to a constructor.

mnm678 · 2021-12-01T20:13:51Z

docs/repository-library-design.md

+  of the repository API: they could be part of Metadata API as well
+
+It would make sense for python-tuf to ship with at least one concrete
+Repository implementation: possibly a repo.py look alike. This implementation


It might be nice to have this example concrete implementation support a CLI for demos and such.

Yes. I didn't want to bloat these docs but I see a few potential implementations:

some sort of CLI tool, to be able to generate and modify metadata for demo and testing purposes

an example repository server (a "mini-warehouse"): a focused and opinionated repository example that could maybe include a custom web api for uploading new targets and a tiny developer tool to do the upload. It should be possible to make this into an example that runs out-of-the-box

Both of these could live inside or outside the repo... I'm not against the first one at all but I kind of feel the second one is even more important. The focus on CLI tools in TUF community (where every project seems to be a CLI tool) is not just a good thing IMO: it creates this impression that production repos can be shell scripts calling CLI tools**... which I don't think is such a great idea.

** yes, I realize I've written exactly this myself: don't do as I do, do as I say 😬

Add 1.0.0 announcment document and point to it in main README. TODO: - Commit message - PR (blocks on theupdateframework#1693, theupdateframework#1675, maybe theupdateframework#1700)

jku · 2021-12-07T10:56:37Z

I believe I've addressed the comments so far.

lukpueh

Thanks for addressing the comments, @jku! I think the added information is very helpful. I found some typos and have one general request.

docs/repository-library-design.md

In addition to multiple smaller review fixes: * Explain how the proposed library is minimal: more specific functionality may be added as we get more experience * Explain what a concrete Repository implementation must implement (details are obviously subject to change but this is what the current prototype requires) Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>

jku · 2021-12-07T12:03:46Z

Thanks: all suggestions make a lot of sense. I've moved the "abstract methods that must be implemented" part down to the Repository details section.

lukpueh

Thanks for bearing with me and my many comments, @jku!

coveralls · 2024-12-26T15:24:16Z

Pull Request Test Coverage Report for Build 1524860174

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.
No unchanged relevant lines lost coverage.
Overall coverage increased (+1.0%) to 98.579%

Totals
Change from base Build 1520084662:	1.0%
Covered Lines:	3869
Relevant Lines:	3895

💛 - Coveralls

ADR: Add New repository library design

845f307

Document the decision to build a repository library on top of Metadata API. Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>

joshuagl reviewed Nov 24, 2021

View reviewed changes

kairoaraujo reviewed Nov 30, 2021

View reviewed changes

docs/adr/0010-repository-library-design.md Outdated Show resolved Hide resolved

kairoaraujo reviewed Nov 30, 2021

View reviewed changes

docs/adr/0010-repository-library-design.md Outdated Show resolved Hide resolved

docs/adr/0010-repository-library-design.md Outdated Show resolved Hide resolved

jku force-pushed the add-repo-lib-design-adr branch from b3d1a49 to 79ae764 Compare December 1, 2021 08:28

Include the design doc in repo

bcab2e9

* Also add some new diagrams in the design doc * Fix some issues in ADR Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>

jku force-pushed the add-repo-lib-design-adr branch from 79ae764 to bcab2e9 Compare December 1, 2021 08:32

lukpueh reviewed Dec 1, 2021

View reviewed changes

mnm678 reviewed Dec 1, 2021

View reviewed changes

lukpueh mentioned this pull request Dec 3, 2021

doc: announce 1.0.0 stable release #1706

Merged

3 tasks

lukpueh requested changes Dec 7, 2021

View reviewed changes

docs/repository-library-design.md Outdated Show resolved Hide resolved

docs/repository-library-design.md Outdated Show resolved Hide resolved

docs/repository-library-design.md Outdated Show resolved Hide resolved

docs/repository-library-design.md Outdated Show resolved Hide resolved

jku force-pushed the add-repo-lib-design-adr branch from 4946580 to f6ede42 Compare December 7, 2021 12:01

lukpueh approved these changes Dec 8, 2021

View reviewed changes

lukpueh merged commit b2d8572 into theupdateframework:develop Dec 8, 2021

lukpueh mentioned this pull request Dec 13, 2021

Prepare v0.20.0 #1723

Closed

jku deleted the add-repo-lib-design-adr branch December 30, 2024 09:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ADR: Add New repository library design #1693

ADR: Add New repository library design #1693

jku commented Nov 24, 2021

coveralls commented Nov 24, 2021 •

edited

Loading

joshuagl left a comment

jku commented Nov 30, 2021

kairoaraujo left a comment

jku commented Dec 1, 2021

jku commented Dec 1, 2021

rdimitrov commented Dec 1, 2021

lukpueh left a comment

lukpueh Dec 1, 2021

lukpueh Dec 1, 2021

jku Dec 1, 2021 •

edited

Loading

lukpueh Dec 1, 2021

jku Dec 1, 2021

lukpueh Dec 2, 2021 •

edited

Loading

lukpueh Dec 1, 2021

jku Dec 2, 2021 •

edited

Loading

mnm678 Dec 1, 2021

jku Dec 2, 2021 •

edited

Loading

jku commented Dec 7, 2021

lukpueh left a comment

jku commented Dec 7, 2021

lukpueh left a comment

coveralls commented Dec 26, 2024 •

edited

Loading


		## Design

		![Design: Application and library components](repository-library-design-ownership.jpg)

ADR: Add New repository library design #1693

ADR: Add New repository library design #1693

Conversation

jku commented Nov 24, 2021

coveralls commented Nov 24, 2021 • edited Loading

Pull Request Test Coverage Report for Build 1549291197

💛 - Coveralls

joshuagl left a comment

Choose a reason for hiding this comment

jku commented Nov 30, 2021

kairoaraujo left a comment

Choose a reason for hiding this comment

jku commented Dec 1, 2021

jku commented Dec 1, 2021

rdimitrov commented Dec 1, 2021

lukpueh left a comment

Choose a reason for hiding this comment

lukpueh Dec 1, 2021

Choose a reason for hiding this comment

lukpueh Dec 1, 2021

Choose a reason for hiding this comment

jku Dec 1, 2021 • edited Loading

Choose a reason for hiding this comment

lukpueh Dec 1, 2021

Choose a reason for hiding this comment

jku Dec 1, 2021

Choose a reason for hiding this comment

lukpueh Dec 2, 2021 • edited Loading

Choose a reason for hiding this comment

lukpueh Dec 1, 2021

Choose a reason for hiding this comment

jku Dec 2, 2021 • edited Loading

Choose a reason for hiding this comment

mnm678 Dec 1, 2021

Choose a reason for hiding this comment

jku Dec 2, 2021 • edited Loading

Choose a reason for hiding this comment

jku commented Dec 7, 2021

lukpueh left a comment

Choose a reason for hiding this comment

jku commented Dec 7, 2021

lukpueh left a comment

Choose a reason for hiding this comment

coveralls commented Dec 26, 2024 • edited Loading

Pull Request Test Coverage Report for Build 1524860174

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

coveralls commented Nov 24, 2021 •

edited

Loading

jku Dec 1, 2021 •

edited

Loading

lukpueh Dec 2, 2021 •

edited

Loading

jku Dec 2, 2021 •

edited

Loading

jku Dec 2, 2021 •

edited

Loading

coveralls commented Dec 26, 2024 •

edited

Loading