Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New API: Add a constants module #1241

Closed
MVrachev opened this issue Dec 14, 2020 · 4 comments
Closed

New API: Add a constants module #1241

MVrachev opened this issue Dec 14, 2020 · 4 comments

Comments

@MVrachev
Copy link
Collaborator

Description of issue or feature request:
I think it will be useful to have a constant module in the new API.
This will help us to minimize the errors when writing hardcoded strings.
For example, the "snapshot.json" string is used often in the Timestamp class in tuf/api/metadata.py.

@joshuagl raised awareness for that in this comment:
#1223 (comment)

Current behavior:
Harcoded strings.

Expected behavior:
Use constant containing the string.

@jku
Copy link
Member

jku commented Dec 15, 2020

For the specific case mentioned, could it not be Snapshot.filename in metadata.py as the filename seems to be a specified feature of snapshot metadata? That Snapshot.filename could be a "getter-only" property to underline how it's not supposed to be changed...

EDIT: actually the property might not be a good idea as you can't have class properties... Snapshot.filename (not instantiating Snapshot) only works if filename is a plain class variable. But that's Python: could make it Snapshot.FILENAME to make it look more constant

@MVrachev
Copy link
Collaborator Author

Do we need a separate module other than the one added in #1470?

@MVrachev
Copy link
Collaborator Author

MVrachev commented Oct 4, 2021

@jku any opinions?

@MVrachev
Copy link
Collaborator Author

The Metadata API and the updater implementation have evolved sufficiently that we don't need one central place for constants.
Currently, we write the constants directly in the metadata API module itself and we have a separate one inside the updater here.
If in the future we decide we need one central place or to minimize the length of Metadata API we will create other issues for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants