-
Notifications
You must be signed in to change notification settings - Fork 8.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
[FIX ON MKI] x-pack/.../telemetry_config.ts
#197036
[FIX ON MKI] x-pack/.../telemetry_config.ts
#197036
Conversation
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.
Thank you for doing this! 🧡
I added 2 very minor comments.
@@ -249,6 +249,7 @@ export class CoreAppsService { | |||
this.configService.setDynamicConfigOverrides(persistedOverrides.attributes); | |||
latestOverrideVersion = persistedOverrides.version; | |||
} | |||
this.logger.info('Succeeded in applying persisted dynamic config overrides'); |
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.
I'd move this log info inside the IF to only log when a change is actually applied. Otherwise, we'll log it every poll-interval.
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.
oh that makes perfect sense
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.
import { SupertestWithRoleScopeType } from '@kbn/test-suites-xpack/api_integration/deployment_agnostic/services'; | ||
import { FtrProviderContext } from '../../../ftr_provider_context'; | ||
|
||
export default function telemetryConfigTest({ getService }: FtrProviderContext) { | ||
const roleScopedSupertest = getService('roleScopedSupertest'); | ||
let supertestAdminWithApiKey: SupertestWithRoleScopeType; | ||
let supertestAdminWithCookieCredentials: SupertestWithRoleScopeType; | ||
const retry = getService('retry'); | ||
const retryTimeout = 180 * 1000; |
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.
IMO, even a shorter timeout would be enough (we poll every 10s). 20s should be plenty.
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.
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.
🚀
Ran the typechecker locally. Also ran the tests against local mki and real mki, from my local.
💛 Build succeeded, but was flaky
Failed CI StepsTest FailuresMetrics [docs]
|
Summary
See details: #197009