Move metadata class model de/serialization to sub-package #1279

lukpueh · 2021-02-10T12:51:51Z

Fixes #-
Related to #1270

Description of the changes being introduced by the pull request:
This PR moves de/serialization logic of TUF metadata from the metadata class model (api.metadata) to a newly added serialization sub-package (api.serialization) and adopts the metadata API accordingly, with the goal of making it easier for users to swap de/serialization implementations. In particular this PR adds:

abstract base classes to serialize and deserialize to and from wireline metadata bytes (for transport), plus default json implementations, and
an abstract base class to serialize signed bytes (for signature creation and verification) and a default canonical json implementation.

More infos can be found in ...

ADR0006: Where to implement model serialization #1270, which drafts a corresponding ADR and discusses the pros and cons of this and alternative approaches,
79ce034, whose commit message has very specific design considerations, and
the other commit messages, which are informative too. ;)

Discussion:

The meat of the default json and canonical json de/serializers is the from/to_dict logic, which is currently implemented in a serialization.util module. I wonder if we shouldn't just keep it on the metadata class model instead (we'd only need to revert b786bc0).

Pro from/to_dict as metadata class methods:

better structured because encapsulated on each class
keeps the serialization package minimal
from/to_dict is hardly serialization, which implies bytes, but rather some sort of type casting / conversion, which seems appropriate to be implemented on the class
it would spare us the stupid cyclic imports

Pro *_from/to_dict in serialization sub-package:

might make it easier to copy-paste-customize when creating a custom de/serializer implementation.

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

joshuagl · 2021-02-10T14:34:19Z

tuf/api/metadata.py

+            # Function-scope import to avoid circular dependency. Yucky!!!
+            # TODO: At least move to a _get_default_metadata_serializer helper.
+            from tuf.api.serialization.json import JSONSerializer # pylint: disable=import-outside-toplevel
+            serializer = JSONSerializer(True) # Pass True to compact JSON


Suggested change

serializer = JSONSerializer(True) # Pass True to compact JSON

serializer = JSONSerializer(compact=True)

Using the named argument makes it clearer what's going on and obviates the need for the comment.

MVrachev · 2021-02-15T14:18:18Z

tuf/api/serialization/util.py

+                              Targets)
+
+
+def _get_signed_common_args_from_dict(_dict: Mapping[str, Any]) -> list:


How did you decide to accept typing.Mapping instead of typing.Dict here and elsewhere in this file?

Good question! The typing.Dict docs have the answer:

To annotate arguments it is preferred to use an abstract collection type such as Mapping.

I didn't really question that advice, but it makes sense when you consider the API design principle of returning specific types, while accepting generic types.

joshuagl

Lovely PR, apologies for the delay in getting to review this @lukpueh.

I have a few minor questions and suggestions in-line; mostly around typing annotations, and variable & parameter names.

I also want to point out that the new code we are writing since introducing a new coding style seems to consistently violate the style guide's recommendations around imports, specifically:

Use import statements for packages and modules only, not for individual classes or functions.

New code in tuf.api is fairly consistently importing individual classes and functions from modules. Should we consider whether we want to make a style guide modification at this point?

79ce034, whose commit message has very specific design considerations

Thank you for the detailed commit messages. I wonder whether we might want to capture some of this detail in the ADR?

Discussion:
[snip]
Pro from/to_dict as metadata class methods:

better structured because encapsulated on each class

keeps the serialization package minimal

from/to_dict is hardly serialization, which implies bytes, but rather some sort of type casting / conversion, which seems appropriate to be implemented on the class

it would spare us the stupid cyclic imports

Having reviewed this PR, I am in favour of keeping [from|to]_dict as metadata class methods. Curious to read what others think.

tuf/api/pylintrc

tuf/api/serialization/__init__.py

tuf/api/serialization/json.py

joshuagl · 2021-02-23T12:26:53Z

tuf/api/serialization/json.py

+from securesystemslib.formats import encode_canonical
+
+from tuf.api.metadata import Metadata, Signed
+from tuf.api.serialization import (MetadataSerializer,
+                                   MetadataDeserializer,
+                                   SignedSerializer)


Note: the recently adopted style guide recommends against importing individual classes or functions.

I know. I guess I just kept using the convention that we already used in this file, which we started before fully embracing our new style guide... and because PEP 8 says it's okay.

I agree with Joshua on this one.
We don't have a linter that would automatically catch those mistakes, but it's important we correct each other.
Also, it seems logical to forbid importing of functions and classes.
This way we prevent future problems with naming and namespaces.

joshuagl · 2021-02-23T12:30:05Z

tuf/api/serialization/json.py

+
+    def deserialize(self, raw_data: bytes) -> Metadata:
+        """Deserialize utf-8 encoded JSON bytes into Metadata object. """
+        _dict = json.loads(raw_data.decode("utf-8"))


Nit: could we rename this variable, rather than using the _ trick? Maybe json_dict, json_object (as the return type of json.loads is a Python object), or loaded_json.

joshuagl · 2021-02-23T12:48:23Z

tuf/api/metadata.py

@@ -213,11 +227,15 @@ def sign(self, key: JsonDict, append: bool = False) -> JsonDict:
        return signature


-    def verify(self, key: JsonDict) -> bool:
+    def verify(self, key: JsonDict,
+               serializer: SignedSerializer = None) -> bool:


Same comments as above, serializer is optional and might be better named signed_serializer

joshuagl · 2021-02-23T13:19:18Z

tuf/api/serialization/json.py

-        _dict = json.loads(raw_data.decode("utf-8"))
-        return Metadata.from_dict(_dict)
+        try:
+            _dict = json.loads(raw_data.decode("utf-8"))


Same comment here, with the code moved, can we rename _dict? Possible alternatives include json_object and json_dict.

joshuagl · 2021-02-23T15:20:34Z

tuf/api/metadata.py

@@ -357,6 +261,7 @@ def bump_version(self) -> None:
        self.version += 1


+


Are we aiming for three blank lines between top-level definitions? The style guide suggests two: https://google.github.io/styleguide/pyguide.html#35-blank-lines

I must admit that I follow this secret personal convention:

Based on the guide I use "single blank lines as [I] judge appropriate within functions or methods", but then I use two blank lines between methods, to better distinguish from the single ones, and then three blank lines between top-level definitions, to distinguish from the two blank lines.

I should probably either strictly adhere to the guide, or propose a style guide amendment.

Agreed, we should strictly adhere to the guide or propose an amendment. I'm neither for or against this personal convention, but I do think amendments to the style guide which contradict the "upstream" Google guide should be kept to a minimum.

You are right. Let's play by the rules we chose. :) Sorry for the noise.

joshuagl · 2021-02-23T15:20:54Z

tuf/api/metadata.py

@@ -487,6 +369,7 @@ def update(self, version: int, length: int, hashes: JsonDict) -> None:
        }


+


Same comment on blank lines between top-level definitions.

See #1279 (comment)

joshuagl · 2021-02-23T15:21:04Z

tuf/api/metadata.py

@@ -545,6 +419,7 @@ def update(
            self.meta[metadata_fn]['hashes'] = hashes


+


Same blank lines comment here, too.

See #1279 (comment)

trishankatdatadog · 2021-02-24T17:09:09Z

tuf/api/serialization/util.py

+    """Returns dict representation of 'Signed'-subclass object. """
+    # Dispatch to '*_to_dict'-function based on 'Signed' subclass type.
+    # TODO: Use if/else cascade, if easier to read!
+    return {


Whoa. Clever, but maybe a bit too much magic?

Yes probably. :'(

Agreed, the if/else cascade is uglier but less magical and easier to follow.

lukpueh · 2021-02-24T17:29:21Z

Thanks for the detailed review and great suggestions, @joshuagl! I left a couple of replies inline and will address the rest in code. Regarding...

New code in tuf.api is fairly consistently importing individual classes and functions from modules. Should we consider whether we want to make a style guide modification at this point?

Maybe we should ... also in the light of #1261. Do you think we should get this straight before landing this PR?

Thank you for the detailed commit messages. I wonder whether we might want to capture some of this detail in the ADR?

I tried to capture the gist without making the ADR too verbose. But I'll try to revise the ADR once more. :)

Having reviewed this PR, I am in favour of keeping [from|to]_dict as metadata class methods. Curious to read what others think.

Cool. I'll wait just a bit longer to see if others still want to weigh in.

trishankatdatadog · 2021-02-24T17:31:13Z

Having reviewed this PR, I am in favour of keeping [from|to]_dict as metadata class methods. Curious to read what others think.

Can someone enlighten me as to why? Cos I don't see it yet.

MVrachev · 2021-02-24T17:41:49Z

Having reviewed this PR, I am in favour of keeping [from|to]_dict as metadata class methods. Curious to read what others think.

Can someone enlighten me as to why? Cos I don't see it yet.

My personal feeling is that from_dict functions are logical to be located inside the classes because they create a new instance.
I see them as a second constructor.
About to_dict methods I see them as casting which again feels like it's a functionality logically connected with the classes.

trishankatdatadog · 2021-02-24T18:01:29Z

@MVrachev but are they useful to other (de)serializers?

MVrachev · 2021-02-24T20:05:20Z

@MVrachev but are they useful to other (de)serializers?

I am not sure where are the [from|to]_dict functions located relate to the other (de)serializers.
Could you elaborate more on this @trishankatdatadog?

jku · 2021-02-25T10:41:15Z

@MVrachev but are they useful to other (de)serializers?

I am not sure where are the [from|to]_dict functions located relate to the other (de)serializers.
Could you elaborate more on this @trishankatdatadog?

The point Trishank is making is that they look like implementation details of the json de/serializer hidden inside metadata.py. And I think we should admit that that's what they are -- e.g. from_dict() is not a generic constructor it's clearly a helper for deserializing json.

On the other hand, the reason Lukas is proposing we leave them in is that it makes the code simpler -- I agree with this as well: the compromise seems reasonable and should not make other de/serializers any harder to implement.

lukpueh · 2021-02-25T15:01:49Z

FYI, just left another comment on #1270 (comment)

joshuagl · 2021-03-02T12:43:00Z

Thanks for the detailed review and great suggestions, @joshuagl! I left a couple of replies inline and will address the rest in code. Regarding...

New code in tuf.api is fairly consistently importing individual classes and functions from modules. Should we consider whether we want to make a style guide modification at this point?

Maybe we should ... also in the light of #1261. Do you think we should get this straight before landing this PR?

No need to block this PR. However, I think we should figure this out and ensure the new code conforms to the style ASAP. Otherwise we risk similar incidents unintentionally contradicting the style guide in future. At the same time as we ensure new code conforms to the style, it would be good to add style checking to CI – and perhaps have a black configuration for folks to ensure their code matches before submission.

The point Trishank is making is that they look like implementation details of the json de/serializer hidden inside metadata.py. And I think we should admit that that's what they are -- e.g. from_dict() is not a generic constructor it's clearly a helper for deserializing json.

On the other hand, the reason Lukas is proposing we leave them in is that it makes the code simpler -- I agree with this as well: the compromise seems reasonable and should not make other de/serializers any harder to implement.

This matches my understanding, and opinion, also. The compromise is pragmatic and reasonable, but let's be sure the comments indicate that those methods are a helper primarily for conversion to/from JSON.

Add sub-package with 3 abstract base classes to: - serialize Metadata objects to bytes (transport) - deserialize Metadata objects from bytes (transport) - serialize Signed objects to bytes (signatures) pylint notes: - configure tox to use api/pylintrc - configure api/pylintrc to allow classes without public methods (default was 2) Design considerations --------------------- - Why not implement de/serialization on metadata classes? -> See ADR0006. - Why use separate classes for serialization and deserialization? -> Some users might only need either one, e.g. client only needs Deserializer. Maybe there are use cases where different implementations are used to serialize and deserialize. - Why use separate classes for Metadata- and Signed-Serialization? -> They require different concrete types, i.e. Metadata and Signed as parameters, and using these specific types seems to make the interface stronger. - Why are de/serialize methods not class/staticmethods? -> In reality we only use classes to namespace and define a type annotated interface, thus it would be enough to make the methods classmethods. However, to keep the de/serialize interface minimal, we move any custom format configuration to the constructor. (See e.g. "compact" for JSONSerializer in subsequent commit). Naming considerations --------------------- - Why de/serialize? -> Implies byte stream as input or output to the function, which is what our interface needs. - Why not marshaling? -> Synonym for serialize but implies transport, would be okay. - Why not encoding? -> Too abstract and too many connotations (character, a/v). - Why not parse? -> Too abstract and no good opposite terms (unparse, write, dump?) Signed-off-by: Lukas Puehringer <[email protected]>

MVrachev · 2021-03-04T08:18:47Z

tox.ini

+    # Use different pylint configs for legacy and new (tuf/api) code
+    # NOTE: Contrary to what the pylint docs suggest, ignoring full paths does
+    # work, unfortunately each subdirectory has to be ignored explicitly.
+    pylint {toxinidir}/tuf --ignore={toxinidir}/tuf/api,{toxinidir}/tuf/api/serialization


Is there a way to ignore all files under a certain folder?
That way you would be able to ignore everything under tuf/api.

Is there a way to ignore all files under a certain folder?

Nope (see my comment). I even looked at the implementation of --ignore and --ignore-patterns.

IIRC there's a corresponding ticket on the pylint issue tracker.

Add serializer.json module with implementations to serialize and deserialize TUF role metadata to and from the JSON wireline format for transportation, and to serialize the 'signed' part of TUF role metadata to the OLPC Canonical JSON format for signature generation and verification. Signed-off-by: Lukas Puehringer <[email protected]>

- Rename Metadata methods: - to_json_file -> to_file - from_json_file -> from_file - Remove Metadata.from_json/to_json - Remove Signed.to_canonical_bytes - Accept optional de/serializer arguments: - from_file (default: JSONDeserializer) - to_file (default: JSONSerializer) - sign, verify (default: CanonicalJSONSerializer) - inline disable pylint cyclic-import checks Signed-off-by: Lukas Puehringer <[email protected]>

Re-raise all errors that happen during de/serialization as custom De/SerializationError. Whilelist 'e', which is idiomatic for error, in api/pylintrc, and inline exempt broad-except, which are okay if re-raised. Signed-off-by: Lukas Puehringer <[email protected]>

Add tuf.api.serialization.util module with functions to convert between TUF metadata class model and the corresponding dictionary representation. These functions replace the corresponding to/from_dict classmethods. Configure api/pylintrc to exempt '_type' from protected member access warning, because the underscore prefix here is only used to avoid name shadowing. Signed-off-by: Lukas Puehringer <[email protected]>

Revert an earlier commit that moved to/from_dict metadata class model methods to a util module of the serialization sub-package. We keep to/from_dict methods on the metadata classes because: - It seems **idiomatic** (see e.g. 3rd-party libaries such as attrs, pydantic, marshmallow, or built-ins that provide default or customizable dict representation for higher-level objects). The idiomatic choice should make usage more intuitive. - It feels better **structured** when each method is encapsulated within the corresponding class, which in turn should make maintaining/modifying/extending the class model easier. - It allows us to remove function-scope imports (see subsequent commit). Caveat: Now that "the meat" of the sub-packaged JSON serializer is implemented on the class, it might make it harder to create a non-dict based serializer by copy-paste-amending the JSON serializer. However, the benefits from above seem to outweigh the disadvantage. See option 5 of ADR0006 for further details (theupdateframework#1270). Signed-off-by: Lukas Puehringer <[email protected]>

Signed-off-by: Lukas Puehringer <[email protected]>

lukpueh · 2021-03-05T12:29:46Z

This could use another review. :)

Following discussions here, in ADR0006: Where to implement model serialization #1270, and elsewhere I reverted the commit that moved to/from_dict methods to a separate module (8e9afc9).
Unfortunately, I was wrong that this frees us from cyclic imports, but I at least updated the corresponding code comments (8858280).
Given that to/from_dict remain on the class, I made an effort to demystify the design of the subclassed 'Signed' class by adding more explicit helpers (c254fd0).
And I tried to largely remove JSON-bias from the metadata class model so that users understand that JSON is only (the default) one of many TUF wireline formats (f421cdc).
Apart from that I addressed code review comments regarding license boilerplate (2f57eb8), the Optional type hint (71a684b), a named argument (0b92da9), argument renames (b934ded) and blank lines (17c25e2), and a linter warning about inconsistent returns (8be8ec1).

joshuagl

Thank you for the updates Lukas. I have a couple of minor nits and one surprise, why are we using six in code that was never intended to run on Python2?

tuf/api/pylintrc

joshuagl · 2021-03-09T12:11:44Z

tuf/api/serialization/__init__.py

+
+class MetadataDeserializer():
+    """Abstract base class for deserialization of Metadata objects. """
+    __metaclass__ = abc.ABCMeta


Note: now that we are only supporting Python3 we can switch to less confusing metaclass syntax, either

import abc class MetadataDeserializer(ABC):

or

import abc class MetadataDeserializer(metaclass=ABCMeta):

we can handle this in a future PR, though.

joshuagl · 2021-03-09T12:16:08Z

tuf/api/serialization/json.py

+                          sort_keys=True).encode("utf-8")
+
+
+class CanonicalJSONSerializer(SignedSerializer):


Do you think it is worth clarifying, via the class name, that this is the OLPC canonical JSON?
You mention it in the header above, but as we have had several adopters surprised that our Canonical JSON isn't compatible with go-tuf's Canonical JSON, I wonder whether it is worth being very explicit?

Counter-argument, I do not like OLPCCanonicalJSONSerializer as a class name...

I agree with both arguments. :D

Another way to avoid ambiguity would be to not call it CanonicalJSONSerializer at all, but something like DefaultSignedSerializer or so? Then people are forced to read the docstring to get more info. At any rate, I will mention OLPC in the class and function docstrings.

joshuagl · 2021-03-09T12:23:00Z

tuf/api/serialization/json.py

@@ -7,6 +7,7 @@

 """
 import json
+import six


Why are we using six in code that was never intended to support Python2?

For raise_from below, which I will change to raise ... from.

joshuagl · 2021-03-09T12:25:02Z

tuf/api/serialization/json.py

+                              sort_keys=True).encode("utf-8")
+
+        except Exception as e: # pylint: disable=broad-except
+            six.raise_from(SerializationError, e)


Python3 syntax for this appears to be

raise SerializationError from e

joshuagl · 2021-03-09T12:25:32Z

tuf/api/serialization/json.py

+            return encode_canonical(signed_dict).encode("utf-8")
+
+        except Exception as e: # pylint: disable=broad-except
+            six.raise_from(SerializationError, e)


Let's not use six here

raise SerializationError from e

joshuagl · 2021-03-09T12:29:11Z

tuf/api/metadata.py

-            # Function-scope import to avoid circular dependency. Yucky!!!
-            # TODO: At least move to a _get_default_signed_serializer helper.
-            from tuf.api.serialization.json import CanonicalJSONSerializer # pylint: disable=import-outside-toplevel
+        if signed_serializer is None:


I think we're missing some of the serializer -> signed_serializer rename in this patch (8858280)? The argument name and the variable assigned below are still serializer

It's the other way around, this line should have gone into b934ded. Apologies for the bad git add --patching. Happy to rebase. Maybe after a final LGTM?

joshuagl · 2021-03-09T13:29:08Z

tuf/api/metadata.py


+    def _common_fields_to_dict(self) -> Dict[str, Any]:


Should this have the @staticmethod decorator also? Throughout the rest of this patch it is not being called on an instance of the class, but on the super() proxy object, much like _common_fields_from_dict().

Can't be static because it needs self, but thanks for pointing out that those two are called on super(). I guess it's less confusing to call them on self, given that they are only called in the parent class. 🤷

Sounds good, thanks.

joshuagl · 2021-03-09T13:55:44Z

tuf/api/serialization/json.py

+            metadata_obj = Metadata.from_dict(json_dict)
+
+        except Exception as e: # pylint: disable=broad-except
+            six.raise_from(DeserializationError, e)


Python3 syntax for this appears to be

raise SerializationError from e

lukpueh · 2021-03-09T16:56:02Z

Thanks for the review! I (hope I) addressed all your comments in the last 3 commits. I chose to leave CanonicalJSONSerializer as is but update the docstrings. Let me know if that works for you.

I'm happy to also fix the minor git history hiccup from #1279 (comment) once you have approved the PR.

joshuagl

Thank you for quickly addressing the review comments.

Thanks for the review! I (hope I) addressed all your comments in the last 3 commits. I chose to leave CanonicalJSONSerializer as is but update the docstrings. Let me know if that works for you.

Last 3 commits look great, thanks. I'm happy to leave CanonicalJSONSerializer. Let's see if we can come up with a 'better' name before we release with an API stability promise.

I'm happy to also fix the minor git history hiccup from #1279 (comment) once you have approved the PR.

That would be good, thanks!

- Try to clarify purpose and remove unimportant TODO note - Use pylint block-level control for shorter lines, see http://pylint.pycqa.org/en/latest/user_guide/message-control.html#block-disables Signed-off-by: Lukas Puehringer <[email protected]>

Use typing.Optional for optional kwargs that default to None. Signed-off-by: Lukas Puehringer <[email protected]>

- Rename _dict to json_dict to avoid wrong semantics of leading underscore. (leading underscore was initially chosen to avoid name shadowing) - Rename 'serializer' argument of type 'SignedSerializer' to 'signed_serializer', to distinguish from 'serializer' argument of type 'MetadataSerializer'. Signed-off-by: Lukas Puehringer <[email protected]>

Signed-off-by: Lukas Puehringer <[email protected]>

Clarify that the TUF metadata class model is not bound to a JSON wireline format by: - re-wording module, class and method docstrings and code comments to add details about custom and default serialization and the purpose of from/to_dict methods, and - removing the 'JsonDict' type annotation -- instead we use generic Mapping[str, Any] for method arguments and strict Dict[str, Any] as return value as suggested in https://docs.python.org/3/library/typing.html#typing.Dict Signed-off-by: Lukas Puehringer <[email protected]>

Prior to this commit the (abstract) 'Signed' base class implemented from/to_dict methods, to be used by any subclass in addition to or instead of a custom from/to_dict method. The design led to some confusion, especially in 'Signed.from_dict' factories, which instantiated subclass objects when called on a subclass, which didn't implement its own 'from_dict' method. This commit demystifies the design, by implementing from/to_dict on all 'Signed' subclasses, and moving common from/to_dict tasks to helper functions in the 'Signed' class. The newly gained clarity and explicitness comes at the cost of slightly more lines of code. Signed-off-by: Lukas Puehringer <[email protected]>

https://github.com/google/styleguide/blob/gh-pages/pyguide.md#35-blank-lines Signed-off-by: Lukas Puehringer <[email protected]>

Signed-off-by: Lukas Puehringer <[email protected]>

tuf.api is not designed for Python 2 compatibility. This commit removes the following stray compatibility constructs in its serialization subpackage: - '__metaclass__ = abc.ABCMeta' - six.raise_from Signed-off-by: Lukas Puehringer <[email protected]>

- Make class docstrings wording consistent. - Emphasize that we use the OLPC Canonical JSON specification. Signed-off-by: Lukas Puehringer <[email protected]>

Call an instance method and a static method that are only defined in a parent class from child instances using self (instance) and cls (static) instead of super(). While this doesn't make a practical difference, the new syntax is probably less confusing to the reader. Signed-off-by: Lukas Puehringer <[email protected]>

lukpueh · 2021-03-10T09:04:40Z

I'm happy to also fix the minor git history hiccup from #1279 (comment) once you have approved the PR.

That would be good, thanks!

Done. Given that the overall diff of the PR didn't change I suppose your approval is still valid. Merging...

joshuagl reviewed Feb 10, 2021

View reviewed changes

MVrachev reviewed Feb 15, 2021

View reviewed changes

lukpueh mentioned this pull request Feb 17, 2021

ADR0006: Where to implement model serialization #1270

Merged

3 tasks

joshuagl mentioned this pull request Feb 18, 2021

Add validation guidelines #1130

Open

MVrachev mentioned this pull request Feb 22, 2021

New metadata API: add MetadataInfo and TargetFile classes #1223

Closed

3 tasks

joshuagl reviewed Feb 23, 2021

View reviewed changes

trishankatdatadog reviewed Feb 24, 2021

View reviewed changes

lukpueh force-pushed the external_serializer branch from b786bc0 to 91fca86 Compare March 3, 2021 17:19

MVrachev reviewed Mar 4, 2021

View reviewed changes

lukpueh added 5 commits March 4, 2021 10:51

lukpueh force-pushed the external_serializer branch from 91fca86 to 37afbdc Compare March 4, 2021 11:54

Add SPDX style license and copyright boilerplate

2f57eb8

Signed-off-by: Lukas Puehringer <[email protected]>

lukpueh force-pushed the external_serializer branch 5 times, most recently from d5fb7ed to 8be8ec1 Compare March 5, 2021 12:02

lukpueh mentioned this pull request Mar 9, 2021

WIP: Client refactor #1291

Merged

17 tasks

joshuagl requested changes Mar 9, 2021

View reviewed changes

lukpueh force-pushed the external_serializer branch from 7e7187e to a223961 Compare March 9, 2021 16:52

joshuagl approved these changes Mar 9, 2021

View reviewed changes

lukpueh added 11 commits March 10, 2021 09:44

Mark kwargs in metadata API methods as Optional

aa8225c

Use typing.Optional for optional kwargs that default to None. Signed-off-by: Lukas Puehringer <[email protected]>

Use named argument instead of clarifying comment

aba6ba3

Signed-off-by: Lukas Puehringer <[email protected]>

Fix blank lines in tuf.api as per styleguide

326d2af

https://github.com/google/styleguide/blob/gh-pages/pyguide.md#35-blank-lines Signed-off-by: Lukas Puehringer <[email protected]>

Fix inconsistent returns in json serializers

ab92ba2

Signed-off-by: Lukas Puehringer <[email protected]>

Re-word api.serializer.json docstrings

a53d68b

- Make class docstrings wording consistent. - Emphasize that we use the OLPC Canonical JSON specification. Signed-off-by: Lukas Puehringer <[email protected]>

lukpueh force-pushed the external_serializer branch from a223961 to ef91964 Compare March 10, 2021 09:00

lukpueh merged commit 3b33deb into theupdateframework:develop Mar 10, 2021

sechkova mentioned this pull request Mar 12, 2021

Remove MetadataWrapper class #1304

Closed

MVrachev mentioned this pull request Apr 6, 2021

New API: Don't hardcode metadata files extension #1333

Open

3 tasks

lukpueh mentioned this pull request Jul 11, 2022

Generic Payload Parser for DSSE in-toto/securesystemslib#3

Closed

3 tasks

	serializer = JSONSerializer(True) # Pass True to compact JSON
	serializer = JSONSerializer(compact=True)

		Targets)


		def _get_signed_common_args_from_dict(_dict: Mapping[str, Any]) -> list:

		@@ -357,6 +261,7 @@ def bump_version(self) -> None:
		self.version += 1

		@@ -487,6 +369,7 @@ def update(self, version: int, length: int, hashes: JsonDict) -> None:
		}

		@@ -545,6 +419,7 @@ def update(
		self.meta[metadata_fn]['hashes'] = hashes

		sort_keys=True).encode("utf-8")


		class CanonicalJSONSerializer(SignedSerializer):

Move metadata class model de/serialization to sub-package #1279

Move metadata class model de/serialization to sub-package #1279

Conversation

lukpueh commented Feb 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshuagl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MVrachev Mar 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukpueh commented Feb 24, 2021

trishankatdatadog commented Feb 24, 2021

MVrachev commented Feb 24, 2021

trishankatdatadog commented Feb 24, 2021

MVrachev commented Feb 24, 2021

jku commented Feb 25, 2021 • edited Loading

lukpueh commented Feb 25, 2021

joshuagl commented Mar 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukpueh commented Mar 5, 2021

joshuagl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukpueh commented Mar 9, 2021

joshuagl left a comment

Choose a reason for hiding this comment

lukpueh commented Mar 10, 2021

lukpueh commented Feb 10, 2021 •

edited

Loading

MVrachev Mar 4, 2021 •

edited

Loading

jku commented Feb 25, 2021 •

edited

Loading