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

Test proposed API to report progress of environment discovery in two phases #19212

Closed
2 tasks done
karrtikr opened this issue May 31, 2022 · 11 comments
Closed
2 tasks done
Labels
testplan-item Test plan item/assignments for upcoming release
Milestone

Comments

@karrtikr
Copy link

karrtikr commented May 31, 2022

Refs: #19103

Complexity: 2

Create Issue


Requirements

Tagging the team who previously tested #19103, apologies for the late tag as I was OOF.

  1. Use pre release build of the python extension.

Test APIs

  • Test the following APIs in IProposedExtensionAPI:

    /**
    * Tracks discovery progress for current list of known environments, i.e when it starts, finishes or any other relevant
    * stage. Note the progress for a particular query is currently not tracked or reported, this only indicates progress of
    * the entire collection.
    */
    readonly onRefreshProgress: Event<ProgressNotificationEvent>;
    /**
    * Returns a promise for the ongoing refresh. Returns `undefined` if there are no active
    * refreshes going on.
    */
    getRefreshPromise(options?: GetRefreshEnvironmentsOptions): Promise<void> | undefined;

  • Copy over contents of https://github.com/microsoft/vscode-python/blob/main/src/client/apiTypes.ts as needed.

  • Discovery is considered finished when all paths are discovered and validated. This API provides a way so other extensions need not wait for validation to finish. For eg. to now get all paths (without waiting for discovery to be finished), we can do:
    Example usage:

const extension = extensions.getExtension('ms-python.python');
if (extension) {
    if (!extension.isActive) {
        await extension.activate();
    }
    const api: IExtensionApi & IProposedExtensionAPI = extension.exports as IExtensionApi & IProposedExtensionAPI;
    if (api.environment) {
        proposedApi.environment.refreshEnvironment();
        // Wait until stage `ProgressReportStage.allPathsDiscovered` has been reached.
        await api.environment.getRefreshPromise({ stage: ProgressReportStage.allPathsDiscovered });
        const envPaths = api.environment.getEnvironmentPaths(); // Verify if all paths have discovered.
    }
}
@karrtikr karrtikr added the testplan-item Test plan item/assignments for upcoming release label May 31, 2022
@rchiodo
Copy link

rchiodo commented May 31, 2022

Not sure if this is a bug or not, but the progress for allPathsDiscovered isn't any faster than the full refresh. It takes 30 seconds for allPathsDiscovered and almost immediately after that it says finished.

@rchiodo rchiodo removed their assignment May 31, 2022
@karrtikr
Copy link
Author

karrtikr commented May 31, 2022

It is probably because validation has already been run for the session, in that case validating it again takes no time, are you querying this API at startup?

For example for me it's:

Can you try to trigger a refresh manually just before fetching "refresh promise"? I have updated the example code:

console.time('All paths found');
console.time('Discovery finishes');
proposedApi.environment.refreshEnvironment();
proposedApi.environment.getRefreshPromise({ stage: ProgressReportStage.allPathsDiscovered })?.then(() => {
    console.timeLog('All paths found');
});

proposedApi.environment.getRefreshPromise()?.then(() => {
    console.timeLog('Discovery finishes');
});

@rchiodo
Copy link

rchiodo commented May 31, 2022

I was doing that before (well with 'clearCache' : true) but it makes not difference.

30 seconds or so to get all the paths
Validation is immediate after that.

@rchiodo
Copy link

rchiodo commented May 31, 2022

How did you get the 'Conda done' message? Is that just internal to the python extension?

@karrtikr
Copy link
Author

Yes atm it's internal to the extension, we discovered it's almost the same time as "all paths found" and so wasn't worth exposing.

Validation is immediate after that.

It maybe the case the validation already finished while waiting for all paths to be discovered (within those 30 seconds), which is why it returns immediately.

@joaomoreno joaomoreno added this to the May 2022 milestone Jun 1, 2022
@karrtikr
Copy link
Author

karrtikr commented Jun 1, 2022

It maybe the case the validation already finished while waiting for all paths to be discovered (within those 30 seconds), which is why it returns immediately.

If Don also sees the same behavior, I can add some verbose logging to confirm this hypothesis.

@amunger
Copy link

amunger commented Jun 2, 2022

I'll try to test this for @DonJayamanne since he is out sick

@amunger
Copy link

amunger commented Jun 2, 2022

I see something very similar to what Rich described:

info 10:1:42.322: Jupyter Extension: ENVIRONMENT REFRESH - starting discovery
src/platform/logging/consoleLogger.ts:28
info 10:1:51.856: Jupyter Extension: ENVIRONMENT REFRESH - Progress made, stage: allPathsDiscovered
src/platform/logging/consoleLogger.ts:28
info 10:1:52.151: Jupyter Extension: ENVIRONMENT REFRESH - Progress made, stage: discoveryFinished
src/platform/logging/consoleLogger.ts:28
info 10:1:52.159: Jupyter Extension: ENVIRONMENT REFRESH - discovered paths [{"path"...

but the API seems to work

@amunger amunger closed this as completed Jun 2, 2022
@amunger amunger removed their assignment Jun 2, 2022
@karrtikr
Copy link
Author

karrtikr commented Jun 2, 2022

I see, can you please provide the logs as mentioned in the issue template. Set python.logging.level to debug in User settings, reload window, then perform the steps to reproduce before providing the logs.

Output for Python in the Output panel (ViewOutput, change the drop-down the upper-right of the Output panel to Python)

XXX

It contains the timestamps when validation actually completes, so we can verify #19212 (comment).

@rchiodo
Copy link

rchiodo commented Jun 2, 2022

Here's the data:

Console output of my custom extension:

Note: Using the "preview" debug extension
Note: Using the "preview" debug extension
activating extension
starting language server
Congratulations, your extension "test-python-api" is now active!
2022-5-2 16:10:53.49: REFRESH PROGRESS: discoveryStarted
2022-5-2 16:11:17.132: REFRESH PROGRESS: allPathsDiscovered
2022-5-2 16:11:17.266: REFRESH PROGRESS: discoveryFinished

Python log of this happening
pythonlog.txt
:

@karrtikr
Copy link
Author

karrtikr commented Jun 2, 2022

Thanks! So the API seem to be work as expected, but the logs reveals the reason why time taken to discover all paths is so much. It's because of us running conda run to get executables for conda envs without python, which can probably be avoided. #18934 takes care of it.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
testplan-item Test plan item/assignments for upcoming release
Projects
None yet
Development

No branches or pull requests

5 participants