Support app-specific user-agents #2612
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* application user-agent can be set with UpdaterConfig object * Setting will affect the default fetcher only * the application user-agent will be prefixed to the ngclient default user-agent Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
|
I did not write a test as I couldn't figure out a reasonable way to do it... but I did test this manually: both default and modified user-agent are as expected. |
Pull Request Test Coverage Report for Build 8783571150Details
💛 - Coveralls |
lukpueh
approved these changes
Apr 22, 2024
There was a problem hiding this comment.
Looks good. The purist in me leans towards option 2 in #2611, but your approach seems reasonable enough, and smaller API is definitely a good argument.
Re testing:
What speaks against adding a small case to test_updater_ng (or test_fetcher_ng), which creates updaters (or fetchers) with and without the new setting, does a refresh (or fetch) and checks if the the session headers contains the expected string?
| supported_envelopes = [EnvelopeType.METADATA, EnvelopeType.SIMPLE] | ||
| if self.config.envelope_type not in supported_envelopes: | ||
| raise ValueError( | ||
| f"config: envelope_type must be one of {supported_envelopes}, " | ||
| f"got '{self.config.envelope_type}'" | ||
| ) | ||
|
|
||
| # Read trusted local root metadata | ||
| data = self._load_local_metadata(Root.type) |
There was a problem hiding this comment.
- it had to be moved below self.config and self._fetcher setting anyway and
- seemed more sensible to handle all of configuration first, then load local metadata
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
|
Added the suggested test, that seems reasonable |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2611