-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
refactor(server): use RuntimeConfigModel on Runtime service #10036
base: 02-08-feat_server_impl_runtimeconfigmodel
Are you sure you want to change the base?
refactor(server): use RuntimeConfigModel on Runtime service #10036
Conversation
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
7c80c35
to
e92a1c5
Compare
8e761a8
to
87450be
Compare
87450be
to
7070771
Compare
e92a1c5
to
14064ca
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 02-08-feat_server_impl_runtimeconfigmodel #10036 +/- ##
=============================================================================
- Coverage 54.02% 53.99% -0.03%
=============================================================================
Files 2323 2323
Lines 107064 107008 -56
Branches 17736 17734 -2
=============================================================================
- Hits 57843 57782 -61
- Misses 47892 47897 +5
Partials 1329 1329
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
14064ca
to
470d695
Compare
7070771
to
f34cea6
Compare
470d695
to
841df8c
Compare
7f53769
to
f712fe0
Compare
841df8c
to
886fa35
Compare
f712fe0
to
879dc03
Compare
886fa35
to
a3c58bd
Compare
879dc03
to
cb2a226
Compare
a3c58bd
to
88c9ebc
Compare
88c9ebc
to
309176f
Compare
af99810
to
0505384
Compare
309176f
to
8a49bc4
Compare
0505384
to
67b19c7
Compare
Page, | ||
Edgeless, | ||
} | ||
|
||
export enum WorkspaceRole { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep the pr clean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I will rewrite it after 0.20 released.
@@ -1,4 +1,4 @@ | |||
import { FlattenedAppRuntimeConfig } from '../config/types'; | |||
import { FlattenedAppRuntimeConfig } from '../../base/config/types'; | |||
|
|||
declare global { | |||
interface Events { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Re: line +5]
time to impl it.
'runtime.init'
'runtime.changed'
instances can keep runtime config in memory to avoid read it every time used. or adjust the runtime when value changed
@OnEvent('runtime.changed')
updateConcurrency(changes: Partial<Runtime>) {
if (changes['job/worker.concurrency']) {
this.worker.concurrency = changes['job/worker.concurrency']
}
}
See this comment inline on Graphite.
deletedAt: null, | ||
}, | ||
}); | ||
const existingConfigs = await this.models.runtimeConfig.findManyByModule(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move it to always run data migration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and there was a bug regarding this. when beta introduces a new config, and stable got redeployed, the new one will be marked deleted
and cause beta down
value: value as any, | ||
deletedAt: null, | ||
}, | ||
const config = await this.models.runtimeConfig.upsert(key, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need a way to define validator on values. basic type validator for String/Number/Boolean, or custom like Semver.valid
for versioning checks
move runtime from base to core
close CLOUD-120