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

Consider skip parameter during preloading of configs on caching #1622

Merged
merged 3 commits into from
Nov 7, 2024

Conversation

tomazpu
Copy link
Contributor

@tomazpu tomazpu commented Nov 7, 2024

What this PR does / Why we need it:

Skip caching for configs of type classic and setting that have the skip parameter set to true. As the configs won't be deployed there is no need to chache them. With this being skipped, we also prevent performance degradation compared to the previous version.

Special notes for your reviewer:

Does this PR introduce a user-facing change?

@tomazpu tomazpu requested a review from a team as a code owner November 7, 2024 07:57
@tomazpu tomazpu self-assigned this Nov 7, 2024
@tomazpu tomazpu added run-e2e-test Manually trigger the E2E tests for reviewed PRs release-notes This feature/fix should be mentioned in release notes labels Nov 7, 2024
Copy link

github-actions bot commented Nov 7, 2024

E2E Test Results

    4 files   -   1    270 suites   - 134   44m 14s ⏱️ - 29m 10s
2 019 tests  -   1  2 017 ✅  -   1  2 💤 ±0  0 ❌ ±0 
2 134 runs   - 114  2 132 ✅  - 114  2 💤 ±0  0 ❌ ±0 

Results for commit 82890e7. ± Comparison against base commit 7ec89f8.

This pull request removes 2 and adds 1 tests. Note that renamed tests count towards both.
github.com/dynatrace/dynatrace-configuration-as-code/v2/cmd/monaco/integrationtest/v2 ‑ TestPaginationClassic
github.com/dynatrace/dynatrace-configuration-as-code/v2/cmd/monaco/integrationtest/v2 ‑ TestPaginationPlatform
github.com/dynatrace/dynatrace-configuration-as-code/v2/pkg/deploy ‑ Test_gatherPreloadConfigTypeEntries_WithSkipParam

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Nov 7, 2024

Unit Test Results

1 904 tests  +1   1 903 ✅ +1   1m 58s ⏱️ ±0s
  134 suites ±0       1 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit bfe13fd. ± Comparison against base commit 7ec89f8.

♻️ This comment has been updated with latest results.

jskelin
jskelin previously approved these changes Nov 7, 2024
@@ -359,3 +359,50 @@ func Test_ScopedConfigsAreNotCached(t *testing.T) {
})
}
}

func Test_gatherPreloadConfigTypeEntries_WithSkipParam(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is your opinion if we divide this test into 3 subtests:

  • skip = true
  • skip = false
  • skip is not defined (aka default value)

Is it going to be more readable?

Laubi
Laubi previously approved these changes Nov 7, 2024
Copy link

sonarcloud bot commented Nov 7, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@tomazpu tomazpu merged commit 216f434 into main Nov 7, 2024
10 of 11 checks passed
@tomazpu tomazpu deleted the fix/preload/caching-issue branch November 7, 2024 09:34
@tomazpu tomazpu changed the title Consider Skip parameter during preload caching Consider skip parameter during preloading of configs on caching Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes This feature/fix should be mentioned in release notes run-e2e-test Manually trigger the E2E tests for reviewed PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants