Improved telemetry for Native Locator #23664
Conversation
DonJayamanne
added
no-changelog
| @@ -106,13 +106,6 @@ class EnvironmentInfoService implements IEnvironmentInfoService { | |||
| } | |||
|
|
|||
| const deferred = createDeferred<InterpreterInformation>(); | |||
| const info = EnvironmentInfoService.getInterpreterInfo(env); | |||
| if (info !== undefined) { | |||
| this.cache.set(normCasePath(interpreterPath), deferred); | |||
There was a problem hiding this comment.
Separate PR for this #23662
Made the changes here to get stuff working, Found the above issue while working on this PR
| } | ||
| } | ||
|
|
||
| if (env.version.major > -1 && env.version.minor > -1 && env.version.micro > -1 && env.location) { |
| // However in the extension we treat this as an environment with an executable that can be `python` or `<fully resolved path to what we think will be the Python exe>`. | ||
| // However native locator will not return exes. Even though the env is detected. | ||
| // For those cases we'll look at the sysprefix. | ||
| let exe = env.executable.filename || ''; |
There was a problem hiding this comment.
See here #23565 (comment)
| @@ -262,129 +265,435 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection | |||
| private telemetrySentOnce = false; | |||
|
|
|||
| private async sendTelemetry(query: PythonLocatorQuery | undefined, stopWatch: StopWatch) { | |||
| if (!query && !this.hasRefreshFinished(query)) { | |||
There was a problem hiding this comment.
Sending telemetry should have absolutely no impact on hasRefreFinished or even this.hasRefreshFinishedForQuery.set(query, true);
Hence moved the implementation code into a separate method sendTelemetryImpl.
That made the code simpler and also fixed the failing test.
I guess the problem was this.telemetrySentOnce was bailing early and not initializing the necessary flags (though not sure how this worked before this PR),
Either way, separating the two (i.e. separating the two concerns) fixes it
There was a problem hiding this comment.
Ideally this method should be renamed and hasRefrehFinished should not be done here, at least IMHO, as this is telemetry.
DonJayamanne
force-pushed
the
betterNativeTelemetry2
branch
from
f0b4e82 to
d92c2ed
Compare
No description provided.