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

APPS-1308 Update Apply config endpoint #243

Merged
merged 44 commits into from
Oct 15, 2024
Merged

APPS-1308 Update Apply config endpoint #243

merged 44 commits into from
Oct 15, 2024

Conversation

korotkov-aerospike
Copy link
Contributor

@korotkov-aerospike korotkov-aerospike commented Oct 10, 2024

  1. on app startup moved instantiation on top level, reducing number of params
  2. configuration manager return model.Config on read
  3. make configuration manager impls private
  4. (most important) on each CRUD operation call changeConfig method, that updates config, saves it and applies.
  5. on reschedule leave ad-hoc jobs

Verified

This commit was signed with the committer’s verified signature. The key has expired.
tvdeyen Thomas von Deyen
@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2024

Codecov Report

Attention: Patch coverage is 3.77358% with 51 lines in your changes missing coverage. Please review.

Project coverage is 27.75%. Comparing base (5006ea7) to head (764a734).

Files with missing lines Patch % Lines
pkg/service/config_applier.go 0.00% 49 Missing ⚠️
pkg/service/restore_run.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##               v2     #243      +/-   ##
==========================================
- Coverage   29.97%   27.75%   -2.23%     
==========================================
  Files          37       39       +2     
  Lines        1948     2104     +156     
==========================================
  Hits          584      584              
- Misses       1281     1437     +156     
  Partials       83       83              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@korotkov-aerospike korotkov-aerospike changed the title Apply config APPS-1308 Update Apply config endpoint Oct 10, 2024
@korotkov-aerospike korotkov-aerospike marked this pull request as ready for review October 14, 2024 19:38
Copy link
Member

@reugn reugn left a comment

Choose a reason for hiding this comment

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

Does the refactoring require regenerating OpenAPI spec?

@korotkov-aerospike korotkov-aerospike merged commit f35916d into v2 Oct 15, 2024
4 checks passed
@korotkov-aerospike korotkov-aerospike deleted the applyConfig branch October 15, 2024 11:36
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

Successfully merging this pull request may close these issues.

None yet

3 participants