You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
[edited on 12/20/2021: Mention ".json" bias] [edited on 11/10/2021: Point to #1230]
Description of issue or feature request:
The Metadata API classes for top-level roles Targets, Timestamp and Snapshot each have an update method. They have been criticised in several places (#1193 (comment) pp, #1223 (comment) pp, #1620 (comment)).
More specifically the following doubts have been expressed:
[e.g. Targets.update()] inserts or replaces the TargetFile... but a Targets handles both TargetFiles and delegations: what if I wanted to modify delegations instead? is it obvious that TargetFile needs a function but delegations do not? - Add missing method args docs in metadata API #1620 (comment)
[e.g. Snapshot.update()] is opinionated in regards to the file extension of the targets metadata to be updated with, i.e. it assumes that it is .json. The rest of the metadata API also often defaults to json, but allows a caller to override, which is not the case here.
Current behavior:
purpose of method is unclear
method name is ambiguous
Expected behavior:
Make a better case for the purpose and names of methods and document it, or replace/remove.
The case for (better named) update methods is that they do allow us to do validation on user input, while inserting things directly to a dictionary does not.
This is a major advantage in an untyped language but, I think, made slightly less important with good static typing support: calling code is still free to insert nonsense into our Metadata data structures, but static typing checks will inform them that they are doing bad things (at least it will once we fix #1633). We may still want to do the input validation but I would argue it's not as critical as it would be otherwise.
Activity
jku commentedon Oct 26, 2021
The case for (better named) update methods is that they do allow us to do validation on user input, while inserting things directly to a dictionary does not.
This is a major advantage in an untyped language but, I think, made slightly less important with good static typing support: calling code is still free to insert nonsense into our Metadata data structures, but static typing checks will inform them that they are doing bad things (at least it will once we fix #1633). We may still want to do the input validation but I would argue it's not as critical as it would be otherwise.
MVrachev commentedon Nov 10, 2021
Issue #1230 and this one are almost the same.
Maybe we can close #1230?
lukpueh commentedon Nov 10, 2021
Added one concern from #1230 to the issue description above. Will close there.
Metadata API: Remove 3 'update' methods + tests
Metadata API: Remove 3 'update' methods + tests
Metadata API: Remove 3 'update' methods + tests