(core) add a python3 button

Summary: This adds a dropdown to the document settings model in staging/dev to set the python engine to Python2 or Python3. The setting is saved in `_grist_DocInfo.documentSettings.engine`.

Test Plan: tested manually for now - separate diff needed to add runsc to jenkins setup and make this testable

Reviewers: dsagal, alexmojaki

Reviewed By: alexmojaki

Differential Revision: https://phab.getgrist.com/D3014

app/client/ui/DocumentSettings.ts

@@ -12,11 +12,14 @@ import {loadMomentTimezone} from 'app/client/lib/imports';
 import {DocInfoRec} from 'app/client/models/DocModel';
 import {DocPageModel} from 'app/client/models/DocPageModel';
 import {testId, vars} from 'app/client/ui2018/cssVars';
+import {select} from 'app/client/ui2018/menus';
 import {saveModal} from 'app/client/ui2018/modals';
 import {buildTZAutocomplete} from 'app/client/widgets/TZAutocomplete';
+import {EngineCode} from 'app/common/DocumentSettings';
+import {GristLoadConfig} from 'app/common/gristUrls';
 import {locales} from "app/common/Locales";
 import {buildCurrencyPicker} from 'app/client/widgets/CurrencyPicker';
-import * as LocaleCurrency from "locale-currency";
+import * as LocaleCurrency from 'locale-currency';
 
 /**
  * Builds a simple saveModal for saving settings.
@@ -26,9 +29,14 @@ export async function showDocSettingsModal(docInfo: DocInfoRec, docPageModel: Do
   return saveModal((ctl, owner) => {
     const timezoneObs = Observable.create(owner, docInfo.timezone.peek());
 
-    const {locale, currency} = docInfo.documentSettingsJson.peek();
+    const docSettings = docInfo.documentSettingsJson.peek();
+    const {locale, currency, engine} = docSettings;
     const localeObs = Observable.create(owner, locale);
     const currencyObs = Observable.create(owner, currency);
+    const engineObs = Observable.create(owner, engine);
+
+    // Check if server supports engine choices - if so, we will allow user to pick.
+    const canChangeEngine = getSupportedEngineChoices().length > 0;
 
     return {
       title: 'Document Settings',
@@ -44,23 +52,37 @@ export async function showDocSettingsModal(docInfo: DocInfoRec, docPageModel: Do
           dom.create(buildCurrencyPicker, currencyObs, (val) => currencyObs.set(val),
             {defaultCurrencyLabel: `Local currency (${LocaleCurrency.getCurrency(l)})`})
         )),
+        canChangeEngine ? [
+          cssDataRow('Engine:'),
+          select(engineObs, getSupportedEngineChoices()),
+        ] : null,
       ],
-      saveFunc: () => docInfo.updateColValues({
-        timezone: timezoneObs.get(),
-        documentSettings: JSON.stringify({
-          ...docInfo.documentSettingsJson.peek(),
-          locale: localeObs.get(),
-          currency: currencyObs.get()
-        })
-      }),
+      // Modal label is "Save", unless engine is changed. If engine is changed, the document will
+      // need a reload to switch engines, so we replace the label with "Save and Reload".
+      saveLabel: dom.text((use) => (use(engineObs) === docSettings.engine) ? 'Save' : 'Save and Reload'),
+      saveFunc: async () => {
+        await docInfo.updateColValues({
+          timezone: timezoneObs.get(),
+          documentSettings: JSON.stringify({
+            ...docInfo.documentSettingsJson.peek(),
+            locale: localeObs.get(),
+            currency: currencyObs.get(),
+            engine: engineObs.get()
+          })
+        });
+        // Reload the document if the engine is changed.
+        if (engineObs.get() !== docSettings.engine) {
+          await docPageModel.appModel.api.getDocAPI(docPageModel.currentDocId.get()!).forceReload();
+        }
+      },
       // If timezone, locale, or currency hasn't changed, disable the Save button.
       saveDisabled: Computed.create(owner,
         (use) => {
-          const docSettings = docInfo.documentSettingsJson.peek();
           return (
             use(timezoneObs) === docInfo.timezone.peek() &&
             use(localeObs) === docSettings.locale &&
-            use(currencyObs) === docSettings.currency
+            use(currencyObs) === docSettings.currency &&
+            use(engineObs) === docSettings.engine
           );
         })
     };
@@ -105,3 +127,9 @@ const cssDataRow = styled('div', `
   margin: 16px 0px;
   font-size: ${vars.largeFontSize};
 `);
+
+// Check which engines can be selected in the UI, if any.
+export function getSupportedEngineChoices(): EngineCode[] {
+  const gristConfig: GristLoadConfig = (window as any).gristConfig || {};
+  return gristConfig.supportEngines || [];
+}

app/common/AsyncCreate.ts

@@ -4,7 +4,7 @@
  * On failure, the result is cleared, so that subsequent calls attempt the creation again.
  *
  * Usage:
- *  this._obj = AsyncCreate<MyObject>(asyncCreateFunc);
+ *  this._obj = new AsyncCreate<MyObject>(asyncCreateFunc);
  *  obj = await this._obj.get();    // calls asyncCreateFunc
  *  obj = await this._obj.get();    // uses cached object if asyncCreateFunc succeeded, else calls it again.
  *

app/common/DocumentSettings.ts

@@ -1,4 +1,11 @@
 export interface DocumentSettings {
   locale: string;
   currency?: string;
+  engine?: EngineCode;
 }
+
+/**
+ * The back-end will for now support at most two engines, a pynbox-backed python2 and
+ * a gvisor-backed python3.
+ */
+export type EngineCode = 'python2' | 'python3';

app/common/gristUrls.ts

@@ -1,5 +1,6 @@
 import {BillingPage, BillingSubPage, BillingTask} from 'app/common/BillingAPI';
 import {OpenDocMode} from 'app/common/DocListAPI';
+import {EngineCode} from 'app/common/DocumentSettings';
 import {encodeQueryParams, isAffirmative} from 'app/common/gutil';
 import {localhostRegex} from 'app/common/LoginState';
 import {LocalPlugin} from 'app/common/plugin';
@@ -426,6 +427,10 @@ export interface GristLoadConfig {
   // If set, enable anonymous sharing UI elements.
   supportAnon?: boolean;
 
+  // If set, allow selection of the specified engines.
+  // TODO: move this list to a separate endpoint.
+  supportEngines?: EngineCode[];
+
   // Max upload allowed for imports (except .grist files), in bytes; 0 or omitted for unlimited.
   maxUploadSizeImport?: number;
 

app/server/lib/ActiveDoc.ts

@@ -26,7 +26,7 @@ import {
   ServerQuery
 } from 'app/common/ActiveDocAPI';
 import {ApiError} from 'app/common/ApiError';
-import {mapGetOrSet, MapWithTTL} from 'app/common/AsyncCreate';
+import {AsyncCreate, mapGetOrSet, MapWithTTL} from 'app/common/AsyncCreate';
 import {
   BulkColValues,
   CellValue,
@@ -38,8 +38,9 @@ import {
 } from 'app/common/DocActions';
 import {DocData} from 'app/common/DocData';
 import {DocSnapshots} from 'app/common/DocSnapshot';
+import {DocumentSettings} from 'app/common/DocumentSettings';
 import {FormulaProperties, getFormulaProperties} from 'app/common/GranularAccessClause';
-import {byteString, countIf} from 'app/common/gutil';
+import {byteString, countIf, safeJsonParse} from 'app/common/gutil';
 import {InactivityTimer} from 'app/common/InactivityTimer';
 import * as marshal from 'app/common/marshal';
 import {UploadResult} from 'app/common/uploads';
@@ -73,7 +74,7 @@ import {DocStorage} from './DocStorage';
 import {expandQuery} from './ExpandedQuery';
 import {GranularAccess, GranularAccessForBundle} from './GranularAccess';
 import {OnDemandActions} from './OnDemandActions';
-import {getLogMetaFromDocSession} from './serverUtils';
+import { getLogMetaFromDocSession, supportsEngineChoices} from './serverUtils';
 import {findOrAddAllEnvelope, Sharing} from './Sharing';
 import cloneDeep = require('lodash/cloneDeep');
 import flatten = require('lodash/flatten');
@@ -129,7 +130,7 @@ export class ActiveDoc extends EventEmitter {
   // result).
   protected _modificationLock: Mutex = new Mutex();
 
-  private readonly _dataEngine: ISandbox;
+  private _dataEngine: AsyncCreate<ISandbox>|undefined;
   private _activeDocImport: ActiveDocImport;
   private _onDemandActions: OnDemandActions;
   private _granularAccess: GranularAccess;
@@ -146,13 +147,14 @@ export class ActiveDoc extends EventEmitter {
   // Timer for shutting down the ActiveDoc a bit after all clients are gone.
   private _inactivityTimer = new InactivityTimer(() => this.shutdown(), Deps.ACTIVEDOC_TIMEOUT * 1000);
   private _recoveryMode: boolean = false;
+  private _shuttingDown: boolean = false;
 
-  constructor(docManager: DocManager, docName: string, options?: {
+  constructor(docManager: DocManager, docName: string, private _options?: {
     safeMode?: boolean,
     docUrl?: string
   }) {
     super();
-    if (options?.safeMode) { this._recoveryMode = true; }
+    if (_options?.safeMode) { this._recoveryMode = true; }
     this._docManager = docManager;
     this._docName = docName;
     this.docStorage = new DocStorage(docManager.storageManager, docName);
@@ -165,22 +167,15 @@ export class ActiveDoc extends EventEmitter {
     // user-defined python code including formula calculations. It maintains all document data and
     // metadata, and applies translates higher-level UserActions into lower-level DocActions.
 
-    // HACK: If doc title as a slug contains "activate-python3-magic", and we are
-    // in an environment with GRIST_EXPERIMENTAL_PLUGINS=1 (dev or staging but not
-    // prod), use Python3.  This is just for experimentation at this point.
-    // TODO: use a less hacky way to say we want to use py3 in a document.
-    const preferredPythonVersion =
-      (options?.docUrl?.match(/activate-python3-magic/) && process.env.GRIST_EXPERIMENTAL_PLUGINS === '1')
-      ? '3' : undefined;
-
-    this._dataEngine = this._docManager.gristServer.create.NSandbox({
-      comment: docName,
-      logCalls: false,
-      logTimes: true,
-      logMeta: {docId: docName},
-      docUrl: options?.docUrl,
-      preferredPythonVersion,
-    });
+    // Creation of the data engine currently needs to be deferred if we support a choice of
+    // engines, since we need to look at the document to see what kind of engine it needs.
+    // If we don't offer a choice, go ahead and start creating the data engine now, so it
+    // is created in parallel to fetching the document from external storage (if needed).
+    // TODO: cache engine requirement for doc in home db so we can retain this parallelism
+    // when offering a choice of data engines.
+    if (!supportsEngineChoices()) {
+      this._getEngine().catch(e => this.logError({client: null}, `engine for ${docName} failed to launch: ${e}`));
+    }
 
     this._activeDocImport = new ActiveDocImport(this);
 
@@ -194,6 +189,8 @@ export class ActiveDoc extends EventEmitter {
 
   public get recoveryMode(): boolean { return this._recoveryMode; }
 
+  public get isShuttingDown(): boolean { return this._shuttingDown; }
+
   public async getUserOverride(docSession: OptDocSession) {
     return this._granularAccess.getUserOverride(docSession);
   }
@@ -312,10 +309,12 @@ export class ActiveDoc extends EventEmitter {
     }
 
     try {
+      const dataEngine = this._dataEngine ? await this._getEngine() : null;
+      this._shuttingDown = true;  // Block creation of engine if not yet in existence.
       await Promise.all([
         this.docStorage.shutdown(),
         this.docPluginManager.shutdown(),
-        this._dataEngine.shutdown()
+        dataEngine?.shutdown()
       ]);
       // The this.waitForInitialization promise may not yet have resolved, but
       // should do so quickly now we've killed everything it depends on.
@@ -1362,7 +1361,9 @@ export class ActiveDoc extends EventEmitter {
       this.logDebug(docSession, `loaded in ${loadMs} ms, InactivityTimer set to ${closeTimeout} ms`);
       return true;
     } catch (err) {
-      this.logWarn(docSession, "_finishInitialization stopped with %s", err);
+      if (!this._shuttingDown) {
+        this.logWarn(docSession, "_finishInitialization stopped with %s", err);
+      }
       this._fullyLoaded = true;
       return false;
     }
@@ -1392,7 +1393,10 @@ export class ActiveDoc extends EventEmitter {
     const now = Date.now();
     if (now >= this._lastMemoryMeasurement + MEMORY_MEASUREMENT_INTERVAL_MS) {
       this._lastMemoryMeasurement = now;
-      await this._dataEngine.reportMemoryUsage();
+      if (this._dataEngine && !this._shuttingDown) {
+        const dataEngine = await this._getEngine();
+        await dataEngine.reportMemoryUsage();
+      }
     }
   }
 
@@ -1427,8 +1431,9 @@ export class ActiveDoc extends EventEmitter {
    * Call a method in the sandbox, without checking the _modificationLock.  Calls to
    * the sandbox are naturally serialized.
    */
-  private _rawPyCall(funcName: string, ...varArgs: unknown[]): Promise<any> {
-    return this._dataEngine.pyCall(funcName, ...varArgs);
+  private async _rawPyCall(funcName: string, ...varArgs: unknown[]): Promise<any> {
+    const dataEngine = await this._getEngine();
+    return dataEngine.pyCall(funcName, ...varArgs);
   }
 
   /**
@@ -1439,6 +1444,45 @@ export class ActiveDoc extends EventEmitter {
   private _pyCall(funcName: string, ...varArgs: unknown[]): Promise<any> {
     return this._modificationLock.runExclusive(() => this._rawPyCall(funcName, ...varArgs));
   }
+
+  private async _getEngine(): Promise<ISandbox> {
+    if (this._shuttingDown) { throw new Error('shutting down, data engine unavailable'); }
+    this._dataEngine = this._dataEngine || new AsyncCreate<ISandbox>(async () => {
+
+      // Figure out what kind of engine we need for this document.
+      let preferredPythonVersion: '2' | '3' = '2';
+
+      // Currently only respect engine preference on experimental deployments (staging/dev).
+      if (process.env.GRIST_EXPERIMENTAL_PLUGINS === '1') {
+        // Careful, migrations may not have run on this document and it may not have a
+        // documentSettings column.  Failures are treated as lack of an engine preference.
+        const docInfo = await this.docStorage.get('SELECT documentSettings FROM _grist_DocInfo').catch(e => undefined);
+        const docSettingsString = docInfo?.documentSettings;
+        if (docSettingsString) {
+          const docSettings: DocumentSettings|undefined = safeJsonParse(docSettingsString, undefined);
+          const engine = docSettings?.engine;
+          if (engine) {
+            if (engine === 'python2') {
+              preferredPythonVersion = '2';
+            } else if (engine === 'python3') {
+              preferredPythonVersion = '3';
+            } else {
+              throw new Error(`engine type not recognized: ${engine}`);
+            }
+          }
+        }
+      }
+      return this._docManager.gristServer.create.NSandbox({
+        comment: this._docName,
+        logCalls: false,
+        logTimes: true,
+        logMeta: {docId: this._docName},
+        docUrl: this._options?.docUrl,
+        preferredPythonVersion,
+      });
+    });
+    return this._dataEngine.get();
+  }
 }
 
 // Helper to initialize a sandbox action bundle with no values.

app/server/lib/Sharing.ts

@@ -246,6 +246,16 @@ export class Sharing {
       // when we come to it. For now we only log skipped envelopes as "alien" in _logActionBundle().
       const ownActionBundle: LocalActionBundle = this._filterOwnActions(localActionBundle);
 
+      // If the document has shut down in the meantime, and this was just a "Calculate" action,
+      // return a trivial result.  This is just to reduce noisy warnings in migration tests.
+      if (this._activeDoc.isShuttingDown && isCalculate) {
+        return {
+          actionNum: localActionBundle.actionNum,
+          retValues: [],
+          isModification: false
+        };
+      }
+
       // Apply the action to the database, and record in the action log.
       if (!trivial) {
         await this._activeDoc.docStorage.execTransaction(async () => {

app/server/lib/sendAppPage.ts

@@ -2,6 +2,7 @@ import {GristLoadConfig} from 'app/common/gristUrls';
 import {isAnonymousUser} from 'app/server/lib/Authorizer';
 import {RequestWithOrg} from 'app/server/lib/extractOrg';
 import {GristServer} from 'app/server/lib/GristServer';
+import {getSupportedEngineChoices} from 'app/server/lib/serverUtils';
 import * as express from 'express';
 import * as fse from 'fs-extra';
 import * as path from 'path';
@@ -35,6 +36,7 @@ export function makeGristConfig(homeUrl: string|null, extra: Partial<GristLoadCo
     singleOrg: process.env.GRIST_SINGLE_ORG,
     pathOnly,
     supportAnon: shouldSupportAnon(),
+    supportEngines: getSupportedEngineChoices(),
     pluginUrl,
     stripeAPIKey: process.env.STRIPE_PUBLIC_API_KEY,
     googleClientId: process.env.GOOGLE_CLIENT_ID,

app/server/lib/serverUtils.ts

@@ -5,6 +5,7 @@ import * as path from 'path';
 import { ConnectionOptions } from 'typeorm';
 import * as uuidv4 from 'uuid/v4';
 
+import {EngineCode} from 'app/common/DocumentSettings';
 import * as log from 'app/server/lib/log';
 import { OpenMode, SQLiteDB } from 'app/server/lib/SQLiteDB';
 import { getDocSessionAccess, getDocSessionUser, OptDocSession } from './DocSession';
@@ -149,3 +150,20 @@ export function getLogMetaFromDocSession(docSession: OptDocSession) {
     ...(client ? client.getLogMeta() : {}),   // Client if present will repeat and add to user info.
   };
 }
+
+/**
+ * Only offer choices of engine on experimental deployments (staging/dev).
+ */
+export function getSupportedEngineChoices(): EngineCode[]|undefined {
+  if (process.env.GRIST_EXPERIMENTAL_PLUGINS === '1') {
+    return ['python2', 'python3'];
+  }
+  return undefined;
+}
+
+/**
+ * Check whether a choice of engine is supported.
+ */
+export function supportsEngineChoices(): boolean {
+  return getSupportedEngineChoices() !== undefined;
+}