Document public API for tuf.api and tuf.ngclient

[fridex]

Description of the changes being introduced by the pull request:

Document public API for tuf.api and tuf.ngclient by declaring __all__.

[fridex]

It looks like this circular import causes the reported issue:

python-tuf/tuf/api/serialization/json.py

Lines 16 to 19 in c6f8b58

	# pylint: disable=cyclic-import
	# ... to allow de/serializing Metadata and Signed objects here, while also
	# creating default de/serializers there (see metadata local scope imports).
	# NOTE: A less desirable alternative would be to add more abstraction layers.

This will also break consumers. Moving to draft, feel free to let me know if this is something worth to have and worth to invest time.

[jku]

I agree that there are cases where our public API is not as well defined as it could be. I don't agree with this premise: Document public API for tuf.api and tuf.ngclient by declaring __all__: I haven't checked any python best practices but this is my understanding: Defining __all__ does not define the public API. It only defines what happens when you do from <mod> import *. Things that are not part of __all__ are 100% still part of the public API, they're just not part of "*".

So:

• I don't object to defining __all__ (I won't use it myself but nothing against it): but this does not define the python-tuf public API
• that said, the default value for from <mod> import * is everything in the module... so is e.g. the ngclient change useful if it changes nothing?
• we could also be more strict about what is public API in api/metadata by first defining __init__.py with the things we consider public, and later making the actual metadata.py submodule internal

[lukpueh]

@jku and I just recently talked about the lack of explicitness in our public API (secure-systems-lab/securesystemslib#456 (comment)). I am all for being more explicit.

[jku]

Maybe a good first step is to file an issue -- as lukas said there is some consensus that API could be more strictly defined -- and maybe make proposal of the actual change (how does the API change? or are we just declaring existing API more specifically?)

[fridex]

__all__ is indeed used with star imports. Also, PEP-8 discusses about using it to be explicit about public interfaces, python docs talk about import statements considering also __all__.

Maybe a good first step is to file an issue

Done in #2235

[jku]

Status update

tuf

I think we don't want anything importable at top level (apart from the modules) and nothing in __all__.

tuf.ngclient

ngclient __init__.py defines the API but the internal modules are not prefixed with an underscore so it's not crystal clear... possible further improvements:

• export TargetFile along with Updater in ngclient/__init__.py (as it's a major part of Updater API and requires the user to do another import for no good reason)
• rename updater.py into _updater.py (abd rename the other files as well) to make the API crystal clear

the first of those seems clearly a good idea, second is debatable: it would break someone who is now doing from tuf.ngclient.updater import Updater. Still think both are likely good ideas.

tuf.api.metadata

TODO: someone could write a proper proposal for a api/metadata/__init__.py (or maybe even api/__init__.py ?!)

tuf.repository

This small component was just added -- I did not pay too much attention on __init__.py yet

[fridex]

Closing this as changes in tuf.ngclient were separated in #2233 and #2279. Relevant discussion continues in #2235.