Skip to content

Commit

Permalink
Language server middleware stop gap for notebooks (microsoft#14522)
Browse files Browse the repository at this point in the history
* Rebase changes onto main

* Change how jupyter extension is downloaded

* Try a different way

* Fix build failure?

* Azure Devops is a pain

* Missing paren

* Just download the file

* Wrong way to build

* Missed file from old branch

* Fix unit tests

* Workaround powershell issue

* Put in correct folder

* Specify full path to vsix

* Directory still not found

* Building wrong bundle

* See why not found

* Invalid syntax in yml

* Fail on error

* VSIX was being cleaned

* copy is not a bash command

* Add jupyter into requirements
  • Loading branch information
rchiodo authored Oct 26, 2020
1 parent ad0f632 commit 4860acd
Show file tree
Hide file tree
Showing 23 changed files with 2,043 additions and 223 deletions.
2 changes: 2 additions & 0 deletions build/ci/templates/globals.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ variables:
npm_config_cache: $(Pipeline.Workspace)/.npm
vmImageMacOS: 'macOS-latest'
TS_NODE_FILES: true # Temporarily enabled to allow using types from vscode.proposed.d.ts from ts-node (for tests).
VSIX_NAME_JUPYTER: ms-toolsai-jupyter-insiders.vsix
VSIX_NAME_JUPYTER_ZIP: ms-toolsai-jupyter-insiders.zip
53 changes: 53 additions & 0 deletions build/ci/templates/test_phases.yml
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ steps:
displayName: 'pip install functional requirements'
condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true'))
# Install smoke test specific requirements
- bash: |
python -m pip install --upgrade -r ./build/smoke-test-requirements.txt
displayName: 'pip install smoke test requirements'
condition: and(succeeded(), contains(variables['TestsToRun'], 'testSmoke'))
# Add CONDA to the path so anaconda works
#
# This task will only run if variable `NeedsPythonFunctionalReqs` is true.
Expand Down Expand Up @@ -445,6 +451,33 @@ steps:
env:
DISPLAY: :10
# Note this section below should download an extension when this code is moved to github actions
# name: Azure Login
# uses: azure/login@v1
# with:
# creds: ${{ secrets.AZURE_CREDENTIALS }}
# name: Download JUPYTER VSIX
# run: az storage blob download --file $(VSIX_NAME_JUPYTER) --account-name pvsc --container-name extension-builds-jupyter --name $(VSIX_NAME_JUPYTER) --auth-mode login
# condition: and(succeeded(), or(contains(variables['TestsToRun'], 'testSmoke'), contains(variables['TestsToRun'], 'testInsiders'))
# Otherwise have to run a powershell task

# Work around https://github.com/actions/virtual-environments/issues/1378
- bash: |
sudo chmod -R 777 /run/user
displayName: 'Fix powershell problem on Ubuntu 20'
condition: ne(variables['Agent.Os'], 'Windows_NT')
# Now actually install
- powershell: |
Invoke-WebRequest -Uri https://pvsc.blob.core.windows.net/extension-builds-jupyter/$(VSIX_NAME_JUPYTER) -OutFile "$(Build.SourcesDirectory)/$(VSIX_NAME_JUPYTER_ZIP)"
if (![System.IO.File]::Exists("$(Build.SourcesDirectory)/$(VSIX_NAME_JUPYTER_ZIP)")) {
Write-Warning "Jupyter extension not downloaded"
throw new Exception("Jupyter extension not downloaded")
}
condition: and(succeeded(), or(contains(variables['TestsToRun'], 'testSmoke'), contains(variables['TestsToRun'], 'testInsiders')))
failOnStderr: true
displayName: 'Download Jupyter VSIX'
# Run the smoke tests.
#
# This task only runs if the string 'testSmoke' exists in variable `TestsToRun`.
Expand All @@ -461,11 +494,31 @@ steps:
npm run updateBuildNumber -- --buildNumber $BUILD_BUILDID
npm run package
npx tsc -p ./
cp $(VSIX_NAME_JUPYTER_ZIP) $(VSIX_NAME_JUPYTER)
node --no-force-async-hooks-checks ./out/test/smokeTest.js
displayName: 'Run Smoke Tests'
condition: and(succeeded(), contains(variables['TestsToRun'], 'testSmoke'))
env:
DISPLAY: :10
INSTALL_JUPYTER_EXTENSION: true
# Run the insiders tests.
#
# This task only runs if the string 'testInsiders' exists in variable `TestsToRun`.
#
- bash: |
npm run prePublish
cp $(VSIX_NAME_JUPYTER_ZIP) $(VSIX_NAME_JUPYTER)
node ./out/test/standardTest.js
displayName: 'Run Tests with Insiders'
condition: and(succeeded(), contains(variables['TestsToRun'], 'testInsiders'))
env:
DISPLAY: :10
INSTALL_JUPYTER_EXTENSION: true
INSTALL_PYLANCE_EXTENSION: true
VSC_PYTHON_CI_TEST_VSC_CHANNEL: insiders
TEST_FILES_SUFFIX: insiders.test
CODE_TESTS_WORKSPACE: ./src/testMultiRootWkspc/smokeTests
- task: PublishBuildArtifacts@1
inputs:
Expand Down
3 changes: 3 additions & 0 deletions build/ci/vscode-python-pr-validation.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ stages:
'Smoke':
TestsToRun: 'testSmoke'
NeedsPythonTestReqs: true
'Insiders':
TestsToRun: 'testInsiders'
NeedsPythonTestReqs: true
pool:
vmImage: 'ubuntu-20.04'
steps:
Expand Down
6 changes: 6 additions & 0 deletions build/smoke-test-requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# List of requirements for smoke tests (they will attempt to run a kernel)
jupyter
numpy
matplotlib
pandas
livelossplot
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1935,6 +1935,7 @@
},
"scripts": {
"package": "gulp clean && gulp prePublishBundle && vsce package -o ms-python-insiders.vsix",
"prePublish": "gulp clean && gulp prePublishNonBundle",
"compile": "tsc -watch -p ./",
"compiled": "deemon npm run compile",
"kill-compiled": "deemon --kill npm run compile",
Expand All @@ -1956,7 +1957,8 @@
"testJediLSP": "node ./out/test/languageServers/jedi/lspSetup.js && cross-env CODE_TESTS_WORKSPACE=src/test VSC_PYTHON_CI_TEST_GREP='Language Server:' node ./out/test/testBootstrap.js ./out/test/standardTest.js && node ./out/test/languageServers/jedi/lspTeardown.js",
"testMultiWorkspace": "node ./out/test/testBootstrap.js ./out/test/multiRootTest.js",
"testPerformance": "node ./out/test/testBootstrap.js ./out/test/performanceTest.js",
"testSmoke": "node ./out/test/smokeTest.js",
"testSmoke": "cross-env INSTALL_JUPYTER_EXTENSION=true \"node ./out/test/smokeTest.js\"",
"testInsiders": "cross-env VSC_PYTHON_CI_TEST_VSC_CHANNEL=insiders INSTALL_PYLANCE_EXTENSION=true TEST_FILES_SUFFIX=insiders.test CODE_TESTS_WORKSPACE=src/testMultiRootWkspc/smokeTests \"node ./out/test/standardTest.js\"",
"lint-staged": "node gulpfile.js",
"lint": "tslint src/**/*.ts -t verbose",
"prettier-fix": "prettier 'src/**/*.ts*' --write && prettier 'build/**/*.js' --write",
Expand Down
8 changes: 3 additions & 5 deletions src/client/activation/jedi/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { inject, injectable, named } from 'inversify';

import { ICommandManager } from '../../common/application/types';
import { traceDecorators } from '../../common/logger';
import { IConfigurationService, IDisposable, IExperimentsManager, Resource } from '../../common/types';
import { IDisposable, Resource } from '../../common/types';
import { debounceSync } from '../../common/utils/decorators';
import { EXTENSION_ROOT_DIR } from '../../constants';
import { IServiceContainer } from '../../ioc/types';
Expand Down Expand Up @@ -39,8 +39,6 @@ export class JediLanguageServerManager implements ILanguageServerManager {
@inject(ILanguageServerAnalysisOptions)
@named(LanguageServerType.Jedi)
private readonly analysisOptions: ILanguageServerAnalysisOptions,
@inject(IExperimentsManager) private readonly experimentsManager: IExperimentsManager,
@inject(IConfigurationService) private readonly configService: IConfigurationService,
@inject(ICommandManager) commandManager: ICommandManager
) {
this.disposables.push(
Expand Down Expand Up @@ -128,9 +126,9 @@ export class JediLanguageServerManager implements ILanguageServerManager {

const options = await this.analysisOptions.getAnalysisOptions();
options.middleware = this.middleware = new LanguageClientMiddleware(
this.experimentsManager,
this.configService,
this.serviceContainer,
LanguageServerType.Jedi,
() => this.languageServerProxy?.languageClient,
this.lsVersion
);

Expand Down
Loading

0 comments on commit 4860acd

Please sign in to comment.