Remove environment variables from packages (#702)

README.md

@@ -22,6 +22,11 @@ We are still using node 16.x and npm 8.x. If you are [using nvm](https://github.
 - `npm install` : Install all dependencies and automatically bootstrap packages. Should be run before any of the other steps.
 - `npm start`: Start building all packages and watching them (when possible). Use when you're developing, and your changes will be picked up automatically.
 - `npm test`: Start running tests in all packages and watching (when possible). Use when you're developing, and any breaking changes will be printed out automatically.
+
+  Log messages from our log package are disabled by default in tests in [jest.setup.ts](./jest.setup.ts). To change the log level, set the `DH_LOG_LEVEL` env variable. For example, to enable all log messages run `DH_LOG_LEVEL=4 npm test`.
+
+  Note that log messages from other sources such as react prop types will still be printed since they do not go through our logger.
+
 - `npm run build`: Create a production build of all packages. Mainly used by CI when packaging up a production version of the app.
 
 ## Package Overview

jest.setup.ts

@@ -2,6 +2,13 @@ import 'regenerator-runtime/runtime';
 import '@testing-library/jest-dom';
 import 'jest-canvas-mock';
 import './__mocks__/dh-core';
+import Log from '@deephaven/log';
+
+let logLevel = parseInt(process.env.DH_LOG_LEVEL ?? '', 10);
+if (!Number.isFinite(logLevel)) {
+  logLevel = -1;
+}
+Log.setLogLevel(logLevel);
 
 // disable annoying dnd-react warnings
 // eslint-disable-next-line @typescript-eslint/no-explicit-any

packages/code-studio/.env

@@ -12,6 +12,7 @@ REACT_APP_ENABLE_LOG_PROXY=true
 REACT_APP_SUPPORT_LINK=https://github.com/deephaven/web-client-ui/
 REACT_APP_DOCS_LINK=https://deephaven.io/core/docs/
 BUILD_PATH=./build
+REACT_APP_LOG_LEVEL=2
 
 # Plugin URLs
 ## Path to plugins that load/render as a component (e.g. Table plugins)

packages/code-studio/.env.development

@@ -8,6 +8,7 @@ REACT_APP_CORE_API_URL=http://localhost:10000/jsapi
 PUBLIC_URL=/
 REACT_APP_INTERNAL_COMPONENT_PLUGINS=ExamplePlugin
 PORT=4000
+REACT_APP_LOG_LEVEL=4
 
 # This converts all eslint errors to warnings in dev
 # If they are left as errors, the dev bundle will fail to build

packages/code-studio/README.md

@@ -30,18 +30,12 @@ One common setup is to override the API server URL to point to another server. F
 REACT_APP_CORE_API_URL=https://www.myserver.com/notebooks
 ```
 
-### DEEPHAVEN_LOG_LEVEL
+### REACT_APP_LOG_LEVEL
 
-Printing detailed logs when debugging can be handy. To enable the highest level of logging, set the log level in your `.env.development.local` file:
+Printing detailed logs when debugging can be handy. The highest level of logging is already set in your `.env.development` file. You can change it in `.env.development.local` if desired.
 
 ```shell
-DEEPHAVEN_LOG_LEVEL=4
-```
-
-When running the unit tests, you often do not want any logs to be printed out. To disable the logs while running the unit tests, set the log level in your `.env.test.local` file:
-
-```shell
-DEEPHAVEN_LOG_LEVEL=-1
+REACT_APP_LOG_LEVEL=2 # Warn/Error
 ```
 
 See [@deephaven/log](../log) for more details on the logger.

packages/code-studio/src/log/LogInit.ts

@@ -12,13 +12,15 @@ export const logProxy = new LogProxy();
 export const logHistory = new LogHistory(logProxy);
 
 export default function logInit(): void {
+  Log.setLogLevel(parseInt(process.env.REACT_APP_LOG_LEVEL ?? '', 10));
+
   if (process.env.REACT_APP_ENABLE_LOG_PROXY === 'true') {
     logProxy.enable();
     logHistory.enable();
   }
 
-  if (process.env.NODE_ENV === 'development' && window) {
-    // In development environment, expose the default logger so that log level can be changed dynamically
+  if (window) {
+    // Expose the default logger so that log level can be changed dynamically
     window.DHLog = Log;
     window.DHLogProxy = logProxy;
     window.DHLogHistory = logHistory;

packages/code-studio/src/main/AppMainContainer.jsx

@@ -683,7 +683,15 @@ export class AppMainContainer extends Component {
           />
           <ChartPlugin hydrate={this.hydrateChart} />
           <ChartBuilderPlugin />
-          <ConsolePlugin hydrateConsole={AppMainContainer.hydrateConsole} />
+          <ConsolePlugin
+            hydrateConsole={AppMainContainer.hydrateConsole}
+            notebooksUrl={
+              new URL(
+                `${process.env.REACT_APP_NOTEBOOKS_URL}/`,
+                window.location
+              ).href
+            }
+          />
           <FilterPlugin />
           <PandasPlugin hydrate={this.hydratePandas} />
           <MarkdownPlugin />

packages/console/src/common/ConsoleConstants.ts

@@ -3,15 +3,11 @@ class ConsoleConstants {
    * Map of language keys to their display names
    */
   static get LANGUAGE_MAP(): Map<string, string> {
-    try {
-      return new Map(
-        process.env.REACT_APP_SESSION_LANGUAGES?.split(',')
-          .map(str => str.split('='))
-          .map(([language, displayName]) => [language, displayName ?? language])
-      );
-    } catch {
-      return new Map<never, never>();
-    }
+    return new Map([
+      ['python', 'Python'],
+      ['groovy', 'Groovy'],
+      ['scala', 'Scala'],
+    ]);
   }
 }
 

packages/console/src/common/ConsoleUtils.ts

@@ -1,8 +1,5 @@
 import ShellQuote, { ParseEntry, ControlOperator } from 'shell-quote';
 import dh, { VariableTypeUnion } from '@deephaven/jsapi-shim';
-import Log from '@deephaven/log';
-
-const log = Log.module('FigureChartModel');
 
 class ConsoleUtils {
   static hasComment(arg: ParseEntry): arg is { comment: string } {
@@ -55,20 +52,6 @@ class ConsoleUtils {
     return `${hours}:${minutes}:${seconds}.${milliseconds}`;
   }
 
-  static defaultHost(): string {
-    let defaultHost = window.location.hostname;
-    const apiUrl = process.env.REACT_APP_CORE_API_URL;
-    if (apiUrl != null) {
-      try {
-        const url = new URL(apiUrl);
-        defaultHost = url.hostname;
-      } catch (error: unknown) {
-        log.error('API_URL failed', error);
-      }
-    }
-    return defaultHost;
-  }
-
   static isTableType(type: VariableTypeUnion): boolean {
     return type === dh.VariableType.TABLE || type === dh.VariableType.TREETABLE;
   }

packages/dashboard-core-plugins/src/ConsolePlugin.tsx

@@ -37,6 +37,7 @@ function isNotebookPanel(
 
 export type ConsolePluginProps = DashboardPluginComponentProps & {
   hydrateConsole?: PanelHydrateFunction;
+  notebooksUrl: string;
 };
 
 export function assertIsConsolePluginProps(
@@ -49,7 +50,14 @@ export const ConsolePlugin = (
   props: Partial<ConsolePluginProps>
 ): JSX.Element => {
   assertIsConsolePluginProps(props);
-  const { id, hydrateConsole, layout, panelManager, registerComponent } = props;
+  const {
+    id,
+    hydrateConsole,
+    layout,
+    panelManager,
+    registerComponent,
+    notebooksUrl,
+  } = props;
   const notebookIndex = useRef(0);
   // Map from file ID to panel ID
   const [openFileMap] = useState(new Map<string, string>());
@@ -316,12 +324,13 @@ export const ConsolePlugin = (
           sessionLanguage,
           panelState,
           isPreview,
+          notebooksUrl,
         },
         title,
         id: panelId,
       };
     },
-    [getNotebookTitle, id]
+    [getNotebookTitle, id, notebooksUrl]
   );
 
   const createNotebook = useCallback(

packages/dashboard-core-plugins/src/panels/NotebookPanel.jsx

@@ -1012,18 +1012,14 @@ NotebookPanel.propTypes = {
     settings: PropTypes.shape({}),
     fileMetadata: PropTypes.shape({}),
   }).isRequired,
-  notebooksUrl: PropTypes.string,
+  notebooksUrl: PropTypes.string.isRequired,
 };
 
 NotebookPanel.defaultProps = {
   isDashboardActive: true,
   isPreview: false,
   session: null,
   sessionLanguage: null,
-  notebooksUrl: new URL(
-    `${process.env.REACT_APP_NOTEBOOKS_URL}/`,
-    window.location
-  ).href,
 };
 
 const mapStateToProps = (state, ownProps) => {

packages/embed-grid/.env

@@ -6,6 +6,7 @@ REACT_APP_OPEN_API_NAME=dh-internal.js
 REACT_APP_VERSION=$npm_package_version
 REACT_APP_NAME=$npm_package_name
 BUILD_PATH=./build
+REACT_APP_LOG_LEVEL=2
 
 # We run our own eslint configuration as part of jest tests in src/test/eslint.test.js
 # We don't need to run it as part of react-scripts, we don't need it running on npm build or npm start

packages/embed-grid/.env.development

@@ -3,6 +3,7 @@
 # However, we still need to specify the jsapi path because otherwise it doesn't work in Chrome
 # https://github.com/facebook/create-react-app/issues/5280
 REACT_APP_CORE_API_URL=http://localhost:10000/jsapi
+REACT_APP_LOG_LEVEL=4
 
 PUBLIC_URL=/
 

packages/embed-grid/src/App.tsx

@@ -11,6 +11,8 @@ import { TableUtils } from '@deephaven/jsapi-utils';
 import Log from '@deephaven/log';
 import './App.scss'; // Styles for in this app
 
+Log.setLogLevel(parseInt(process.env.REACT_APP_LOG_LEVEL ?? '', 10));
+
 const log = Log.module('EmbedGrid.App');
 
 export type Command = 'filter' | 'sort';

packages/log/src/DefaultLogger.ts

@@ -1,45 +1,8 @@
 import Logger from './Logger';
-import LoggerLevel, { DEBUG2, INFO, SILENT } from './LoggerLevel';
-
-/**
- * Get the default log level to use.
- * production mode defaults to INFO.
- * development mode defaults to DEBUG2 (most verbose).
- * test mode defaults to SILENT.
- * Override by setting environment variable `process.env.DEEPHAVEN_LOG_LEVEL`
- * @returns The default log level to use
- */
-function getDefaultLevel(): number {
-  let envValue: string | undefined = '';
-  try {
-    envValue = process.env.DEEPHAVEN_LOG_LEVEL;
-  } catch {
-    // no-op. Environment without process defined
-  }
-  if (envValue && LoggerLevel[envValue]) {
-    return LoggerLevel[envValue];
-  }
-  const envLevel = parseInt(envValue ?? '', 10);
-  if (!Number.isNaN(envLevel)) {
-    return envLevel;
-  }
-
-  try {
-    if (process.env.NODE_ENV === 'test') {
-      return SILENT;
-    }
-    if (process.env.NODE_ENV === 'development') {
-      return DEBUG2;
-    }
-  } catch {
-    // no-op. Environment without process defined
-  }
-
-  return INFO;
-}
+import { INFO } from './LoggerLevel';
 
 class DefaultLogger extends Logger {
-  constructor(level = getDefaultLevel()) {
+  constructor(level = INFO) {
     super(null, level);
 
     this.modules = {};

packages/log/src/Logger.ts

@@ -38,6 +38,12 @@ class Logger {
   debug2: (...data: unknown[]) => void;
 
   setLogLevel(level: number): void {
+    if (!Number.isFinite(level)) {
+      console.warn(
+        `Expected a number for log level. Received: ${level}. Ignoring`
+      );
+      return;
+    }
     this.level = level;
 
     this.error =