Skip to content

ngclient: client-level constants #1402

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

Closed
sechkova opened this issue May 20, 2021 · 2 comments · Fixed by #1470
Closed

ngclient: client-level constants #1402

sechkova opened this issue May 20, 2021 · 2 comments · Fixed by #1470
Assignees
Labels
backlog Issues to address with priority for current development goals ngclient
Milestone

Comments

@sechkova
Copy link
Contributor

The constants could use some thought: We definitely don't want to keep using tuf.settings, but I guess the question is, should these be instance variables in Updater instead of module level ones:

MAX_ROOT_ROTATIONS = 32
MAX_DELEGATIONS = 32
DEFAULT_ROOT_MAX_LENGTH = 512000  # bytes
DEFAULT_TIMESTAMP_MAX_LENGTH = 16384  # bytes
DEFAULT_SNAPSHOT_MAX_LENGTH = 2000000  # bytes
DEFAULT_TARGETS_MAX_LENGTH = 5000000  # bytes

Maybe it does not hurt if they were instance variables instead. They don't even need to be optional constructor arguments (as initialization does not use any of them: caller can initialize the Updater and then modify the values).
I guess the fanciest option is that constructor takes an optional argument dataclass UpdaterOptions with those attributes ...

Originally posted by @jku in #1396 (comment)

@sechkova sechkova added the experimental-client Items related to the development of a new client (see milestone/8 and theexperimental-client branch) label May 20, 2021
@jku jku added ngclient and removed experimental-client Items related to the development of a new client (see milestone/8 and theexperimental-client branch) labels May 21, 2021
@jku jku changed the title experimental client: client-level constants ngclient: client-level constants May 25, 2021
@sechkova sechkova added the backlog Issues to address with priority for current development goals label May 26, 2021
@sechkova sechkova self-assigned this Jun 23, 2021
@sechkova sechkova added this to the weeks26-27 milestone Jun 23, 2021
@sechkova
Copy link
Contributor Author

I am in favour of the dataclass option because it groups all the configuration at one place and I drafted a proposal based on it: #1470.
There is also RequestsFetcher which needs to be configured on init so I added the configuration as an optional parameter to Updater. However I am not totally sure if this solution makes it convenient enough to customise the configuration and I will be happy to hear some opinions about it.

@trishankatdatadog
Copy link
Member

I am in favour of the dataclass option because it groups all the configuration at one place and I drafted a proposal based on it: #1470.

Me too 🙂

@jku jku closed this as completed in #1470 Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issues to address with priority for current development goals ngclient
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants