refactor(template): Introduce new File and InMemory Templates #1197
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
UnseenWizzard
added
the
run-e2e-test
UnseenWizzard
force-pushed
the
feat/defer-template-load
branch
from
5a647eb to
547de17
Compare
Test Results 2 files ± 0 220 suites ±0 16s ⏱️ ±0s Results for commit 99b1022. ± Comparison against base commit 44d82db. This pull request removes 10 and adds 2 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Laubi
force-pushed
the
feat/defer-template-load
branch
from
ac502f0 to
6127a30
Compare
UnseenWizzard
force-pushed
the
feat/defer-template-load
branch
from
6127a30 to
d2a838a
Compare
UnseenWizzard
added
run-e2e-test
warber
reviewed
Sep 27, 2023
UnseenWizzard
force-pushed
the
feat/defer-template-load
branch
from
d2a838a to
2f21fe1
Compare
jskelin
reviewed
Sep 29, 2023
UnseenWizzard
force-pushed
the
feat/defer-template-load
branch
2 times, most recently
from
7342ab1 to
99b1022
Compare
UnseenWizzard
removed
the
run-e2e-test
jskelin
approved these changes
Oct 3, 2023
UnseenWizzard
force-pushed
the
feat/defer-template-load
branch
from
99b1022 to
7b87e56
Compare
Unit Test Results 2 files ± 0 220 suites ±0 17s ⏱️ ±0s Results for commit a4511cf. ± Comparison against base commit de70797. This pull request removes 10 and adds 2 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
The updated FileBasedTemplate loads file contents only when required, instead of at creation, reducing the long term in-use memory footprint of configurations. The new InMemoryTemplate replaces the Download specific template, as well as some dedicated test implementations of templates. It currently carries an optional filepath, which is used for writing converted templates to the desired target file.
Method signatures are refactored to return now possible teplate read/write errors. Additionally unused coordinate returns are removed.
%q does not only quote the variable's content, but also makes it 'safe'. It's not clearly defined in the golang docs, but backslashes are escaped as well. To properly render the error, we need to quote the string ourselves and print the string using %s
Also fix spelling of Id() to ID()
…er persisted FileBasedTemplate is only used when a config was loaded from a file (in deployments), and these types of Templates will never be passed to the config writer. Only download and conversion will ever persist their configurations, and both create InMemoryTemplates. To simplify the persistence code and don't make it look like we ever persist FileBasedTemplates, the codepath is simply removed. If an unexpected type of Template still get's passed to the Writer, an error is returned. If we ever extend Templates and don't implement persistence logic, we'll get an error. As the interface already includes an error return and we won't notice this on compile, an error was chosen over a hard-exit by panicking.
UnseenWizzard
force-pushed
the
feat/defer-template-load
branch
from
7b87e56 to
a4511cf
Compare
UnseenWizzard
added
the
run-e2e-test
|
Kudos, SonarCloud Quality Gate passed!
|
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.
What this PR does / Why we need it:
The updated FileBasedTemplate loads file contents only when required, instead of at creation, reducing the long term in-use memory footprint
of configurations.
The new InMemoryTemplate replaces the Download specific template, as well as some dedicated test implementations of templates. It currently carries an optional filepath, which is used for writing converted templates to the desired target file. This may be split off.
Special notes for your reviewer:
It might instead make sense to have dedicated template implementations in
Does this PR introduce a user-facing change?
No visible change, template files are loaded from disk later.