From 66f64dab0a3be1376bc3f2113633ff6fb56b0322 Mon Sep 17 00:00:00 2001 From: rchiodo Date: Fri, 5 Apr 2019 15:14:52 -0700 Subject: [PATCH 1/2] Fix running under docker --- .vscode/devContainer.json | 4 ++++ .vscode/launch.json | 3 ++- news/1 Enhancements/5047.md | 1 + package.json | 2 +- src/client/datascience/constants.ts | 10 +++++++++- .../datascience/jupyter/jupyterConnection.ts | 15 ++++++++++++--- .../datascience/jupyter/jupyterExecution.ts | 19 +++++++++++++++++++ 7 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 .vscode/devContainer.json create mode 100644 news/1 Enhancements/5047.md diff --git a/.vscode/devContainer.json b/.vscode/devContainer.json new file mode 100644 index 000000000000..68af2eff5ab8 --- /dev/null +++ b/.vscode/devContainer.json @@ -0,0 +1,4 @@ +{ + "name": "Extension Debugging", + "image": "jupyter/pyspark-notebook" +} diff --git a/.vscode/launch.json b/.vscode/launch.json index 572a53e118d9..d7e5b76c5426 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -16,7 +16,8 @@ "outFiles": [ "${workspaceFolder}/out/**/*" ], - "preLaunchTask": "Compile" + "preLaunchTask": "Compile", + // "devContainer": true // Use this value to debug in containers }, { "name": "Debugger as debugServer", diff --git a/news/1 Enhancements/5047.md b/news/1 Enhancements/5047.md new file mode 100644 index 000000000000..db9fb682e4af --- /dev/null +++ b/news/1 Enhancements/5047.md @@ -0,0 +1 @@ +Support running under docker. diff --git a/package.json b/package.json index 78aaebb0fe17..a18b66a42343 100644 --- a/package.json +++ b/package.json @@ -2398,8 +2398,8 @@ "mocha-junit-reporter": "^1.17.0", "mocha-multi-reporters": "^1.1.7", "node-has-native-dependencies": "^1.0.2", - "node-sass": "^4.11.0", "node-html-parser": "^1.1.13", + "node-sass": "^4.11.0", "nyc": "^13.3.0", "raw-loader": "^0.5.1", "react": "^16.5.2", diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts index d128e70ca7f9..17a8809dc291 100644 --- a/src/client/datascience/constants.ts +++ b/src/client/datascience/constants.ts @@ -48,7 +48,15 @@ export namespace RegExpValues { export const CheckJupyterRegEx = IS_WINDOWS ? /^jupyter?\.exe$/ : /^jupyter?$/; export const PyKernelOutputRegEx = /.*\s+(.+)$/m; export const KernelSpecOutputRegEx = /^\s*(\S+)\s+(\S+)$/; - export const UrlPatternRegEx = /(https?:\/\/[^\s]+)/ ; + // This next one has to be a string because uglifyJS isn't handling the groups. We use named-js-regexp to parse it + // instead. + export const UrlPatternRegEx = '(?https?:\\/\\/)((\\(.+\\s+or\\s+(?.+)\\))|(?[^\\s]+))(?:.+)' ; + export interface IUrlPatternGroupType { + LOCAL: string | undefined; + PREFIX: string | undefined; + REST: string | undefined; + IP: string | undefined; + } export const HttpPattern = /https?:\/\//; export const ExtractPortRegex = /https?:\/\/[^\s]+:(\d+)[^\s]+/; export const ConvertToRemoteUri = /(https?:\/\/)([^\s])+(:\d+[^\s]*)/; diff --git a/src/client/datascience/jupyter/jupyterConnection.ts b/src/client/datascience/jupyter/jupyterConnection.ts index 0d8bd098abde..149f48f62719 100644 --- a/src/client/datascience/jupyter/jupyterConnection.ts +++ b/src/client/datascience/jupyter/jupyterConnection.ts @@ -18,6 +18,10 @@ import { RegExpValues } from '../constants'; import { IConnection } from '../types'; import { JupyterConnectError } from './jupyterConnectError'; +// tslint:disable-next-line:no-require-imports no-var-requires no-any +const namedRegexp = require('named-js-regexp'); +const urlMatcher = namedRegexp(RegExpValues.UrlPatternRegEx); + export type JupyterServerInfo = { base_url: string; notebook_dir: string; @@ -130,14 +134,19 @@ class JupyterConnectionWaiter { // tslint:disable-next-line:no-any private getJupyterURLFromString(data: any) { - const urlMatch = RegExpValues.UrlPatternRegEx.exec(data); - if (urlMatch && !this.startPromise.completed) { + const urlMatch = urlMatcher.exec(data) as any; + const groups = urlMatch.groups() as RegExpValues.IUrlPatternGroupType; + if (urlMatch && !this.startPromise.completed && groups && (groups.LOCAL || groups.IP)) { + // Rebuild the URI from our group hits + const host = groups.LOCAL ? groups.LOCAL : groups.IP; + const uriString = `${groups.PREFIX}${host}${groups.REST}`; + // URL is not being found for some reason. Pull it in forcefully // tslint:disable-next-line:no-require-imports const URL = require('url').URL; let url: URL; try { - url = new URL(urlMatch[0]); + url = new URL(uriString); } catch (err) { // Failed to parse the url either via server infos or the string this.rejectStartPromise(localize.DataScience.jupyterLaunchNoURL()); diff --git a/src/client/datascience/jupyter/jupyterExecution.ts b/src/client/datascience/jupyter/jupyterExecution.ts index 0eec7c30a096..d36d9f14a5dd 100644 --- a/src/client/datascience/jupyter/jupyterExecution.ts +++ b/src/client/datascience/jupyter/jupyterExecution.ts @@ -36,6 +36,7 @@ import { import { JupyterConnection, JupyterServerInfo } from './jupyterConnection'; import { JupyterKernelSpec } from './jupyterKernelSpec'; import { JupyterWaitForIdleError } from './jupyterWaitForIdleError'; +import { execSync } from 'child_process'; enum ModuleExistsResult { NotFound, @@ -351,6 +352,24 @@ export class JupyterExecutionBase implements IJupyterExecution { extraArgs.push('--debug'); } + // Check for a docker situation. + try { + const cgroup = await this.fileSystem.readFile('/proc/self/cgroup'); + if (cgroup.includes('docker')) { + // We definitely need an ip address. + extraArgs.push('--ip'); + extraArgs.push('127.0.0.1'); + + // Now see if we need --allow-root. + const idResults = execSync('id', {encoding: 'utf-8'}); + if (idResults.includes('(root)')) { + extraArgs.push('--allow-root'); + } + } + } catch { + noop(); + } + // Use this temp file and config file to generate a list of args for our command const args: string[] = [...['--no-browser', `--notebook-dir=${tempDir.path}`], ...extraArgs]; From 7c3b9a476263792777c0294b48f48829903a62ee Mon Sep 17 00:00:00 2001 From: rchiodo Date: Fri, 5 Apr 2019 15:32:13 -0700 Subject: [PATCH 2/2] Remove devContainer and check for file existence before /proc/cgroup --- .vscode/devContainer.json | 4 ---- .vscode/launch.json | 3 +-- .../datascience/jupyter/jupyterExecution.ts | 22 ++++++++++--------- 3 files changed, 13 insertions(+), 16 deletions(-) delete mode 100644 .vscode/devContainer.json diff --git a/.vscode/devContainer.json b/.vscode/devContainer.json deleted file mode 100644 index 68af2eff5ab8..000000000000 --- a/.vscode/devContainer.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "name": "Extension Debugging", - "image": "jupyter/pyspark-notebook" -} diff --git a/.vscode/launch.json b/.vscode/launch.json index d7e5b76c5426..572a53e118d9 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -16,8 +16,7 @@ "outFiles": [ "${workspaceFolder}/out/**/*" ], - "preLaunchTask": "Compile", - // "devContainer": true // Use this value to debug in containers + "preLaunchTask": "Compile" }, { "name": "Debugger as debugServer", diff --git a/src/client/datascience/jupyter/jupyterExecution.ts b/src/client/datascience/jupyter/jupyterExecution.ts index d36d9f14a5dd..ceeaa54dc43f 100644 --- a/src/client/datascience/jupyter/jupyterExecution.ts +++ b/src/client/datascience/jupyter/jupyterExecution.ts @@ -354,16 +354,18 @@ export class JupyterExecutionBase implements IJupyterExecution { // Check for a docker situation. try { - const cgroup = await this.fileSystem.readFile('/proc/self/cgroup'); - if (cgroup.includes('docker')) { - // We definitely need an ip address. - extraArgs.push('--ip'); - extraArgs.push('127.0.0.1'); - - // Now see if we need --allow-root. - const idResults = execSync('id', {encoding: 'utf-8'}); - if (idResults.includes('(root)')) { - extraArgs.push('--allow-root'); + if (await this.fileSystem.fileExists('/proc/self/cgroup')) { + const cgroup = await this.fileSystem.readFile('/proc/self/cgroup'); + if (cgroup.includes('docker')) { + // We definitely need an ip address. + extraArgs.push('--ip'); + extraArgs.push('127.0.0.1'); + + // Now see if we need --allow-root. + const idResults = execSync('id', {encoding: 'utf-8'}); + if (idResults.includes('(root)')) { + extraArgs.push('--allow-root'); + } } } } catch {