diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a8e10167e9..1271b61e22 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,15 +3,7 @@ First off, thank you for thinking about contributing! Below you’ll find instructions that will hopefully guide you through how to contribute to, fix, and improve Flyte Console. -## Storybook - -This project has support for [Storybook](https://storybook.js.org/). -Component stories live next to the components they test, in a `__stories__` -directory, with the filename pattern `{Component}.stories.tsx`. - -You can run storybook with `yarn run storybook`, and view the stories at http://localhost:9001. - -## Protobuf and the Network Tab +## Protobuf and Debug Output Communication with the Flyte Admin API is done using Protobuf as the request/response format. Protobuf is a binary format, which means looking at @@ -20,10 +12,8 @@ each network request is logged to the console with it's URL followed by the decoded Protobuf payload. You must have debug output enabled (on by default in development) to see these messages. -## Debug Output - This application makes use of the [debug](https://github.com/visionmedia/debug) -libary to provide namespaced debug output in the browser console. In +libary to provide namespaced **debug output** in the browser console. In development, all debug output is enabled. For other environments, the debug output must be enabled manually. You can do this by setting a flag in localStorage using the console: `localStorage.debug = 'flyte:*'`. Each module in @@ -32,6 +22,96 @@ a single module, you can specify that one specifically (ex. ``localStorage.debug = 'flyte:adminEntity'`` to only see decoded Flyte Admin API requests). +## Storybook + +This project has support for [Storybook](https://storybook.js.org/). +Component stories live next to the components they test, in a `__stories__` +directory, with the filename pattern `{Component}.stories.tsx`. + +You can run storybook with `yarn run storybook`, and view the stories at http://localhost:9001. + +## Feature flags + +We are using our internal feature flag solution to allow continuos integration, +while features are in development. +All flags currently available could be found in [/FeatureFlags/defaultConfig.ts](./src/basics/FeatureFlags/defaultConfig.ts) +file. Most of them under active development, which means we don't guarantee it will work as you expect. + +If you want to add your own flag, you need to add it to both `enum FeatureFlag` and `defaultFlagConfig` +under production section. +Initally all flags must be disabled, meaning you code path should not be executed by default. + +**Example - adding flags:** + +```javascript +enum FeatureFlags { + ... + AddNewPage: 'add-new-page' + UseCommonPath: 'use-common-path' +} + +export const defaultFlagConfig: FeatureFlagConfig = { + ... + 'add-new-page': false, // default/prior behavior doesn't include new page + 'use-common-path': true, // default/prior behavior uses common path +}; +``` + +To use flags in code you need to ensure that the most top level component is wrapped by `FeatureFlagsProvider`. +By default we are wrapping top component in Apps file, so if you do not plan to include +feature flags checks in the `\*.tests.tsx` - you should be good to go. +To check flag's value use `useFeatureFlag` hook. + +**Example - flag usage**: + +```javascript +import { FeatureFlag, useFeatureFlag } from 'basics/FeatureFlags'; + +export function MyComponent(props: Props): React.ReactNode { + ... + const isFlagEnabled = useFeatureFlag(FeatureFlag.AddNewPage); + + return isFlagEnabled ? : null; +} +``` + +During your local development you can either: +* temporarily switch flags value in runtimeConfig as: + ```javascript + let runtimeConfig = { + ...defaultFlagConfig, + 'add-new-page': true, + }; + ``` +* turn flag on/off from the devTools console in Chrome +![SetFeatureFlagFromConsole](https://user-images.githubusercontent.com/55718143/150002962-f12bbe57-f221-4bbd-85e3-717aa0221e89.gif) + +#### Unit tests + +If you plan to test non-default flag value in your unit tests, make sure to wrap your component with `FeatureFlagsProvider`. +Use `window.setFeatureFlag(flag, newValue)` function to set needed value and `window.clearRuntimeConfig()` +to return to defaults. Beware to comment out/remove any changes in `runtimeConfig` during testing; + +```javascript +function TestWrapper() { + return +} + +describe('FeatureFlags', () => { + afterEach(() => { + window.clearRuntimeConfig(); // clean up flags + }); + + it('Test', async () => { + render(); + + window.setFeatureFlag(FeatureFlag.FlagInQuestion, true); + await waitFor(() => { + // check after flag changed value + }); + }); +``` + ## Google Analytics This application makes use of the `react-ga4 `_ diff --git a/README.md b/README.md index a5bdf1c775..92259e84d0 100644 --- a/README.md +++ b/README.md @@ -33,11 +33,12 @@ ## 📦 Install Dependencies Running flyteconsole locally requires [NodeJS](https://nodejs.org) and -[yarn](https://yarnpkg.com). Once these are installed, all of the dependencies -can be installed by running `yarn install` in the project directory. +[yarn](https://yarnpkg.com). Once these are installed, you can run application locally. +For help with installing dependencies look into +[Installation section](https://github.com/flyteorg/flyteconsole/blob/narusina/fflags/CONTRIBUTING.md#-install-dependencies). ## 🚀 Quick Start -1. Follow [Start a Local flyte backend](https://docs.flyte.org/en/latest/getting_started/first_run.html), like: +1. Follow [Start a Local flyte backend](https://docs.flyte.org/en/latest/getting_started/index.html), like: ```bash docker run --rm --privileged -p 30081:30081 -p 30082:30082 -p 30084:30084 cr.flyte.org/flyteorg/flyte-sandbox ``` @@ -110,16 +111,34 @@ docker run -p 8080:8080 -e BASE_URL="/console" -e CONFIG_DIR="/etc/flyte/config" ### Run the server -To start the local development server, run `yarn start`. This will spin up a -Webpack development server, compile all of the code into bundles, and start the -NodeJS server on the default port (3000). All requests to the NodeJS server will -be stalled until the bundles have finished. The application will be accessible +To start the local development server run: +```bash +yarn install # to install node_modules +yarn start # to start application +``` +This will spin up a Webpack development server, compile all of the code into bundles, +and start the NodeJS server on the default port (3000). All requests to the NodeJS server +will be stalled until the bundles have finished. The application will be accessible at http://localhost:3000 (if using the default port). ## 🛠 Development -Please find most information about developement details in [CONTRIBUTING.md](CONTRIBUTING.md) +For continious development we are using: +* **[Protobuf and Debug Output](https://github.com/flyteorg/flyteconsole/blob/narusina/fflags/CONTRIBUTING.md#protobuf-and-debug-output)**. + Protobuf is a binary response/request format, which makes _Network Tab_ hardly useful. + To get more info on requests - use our Debug Output + +* **[Storybook](https://github.com/flyteorg/flyteconsole/blob/narusina/fflags/CONTRIBUTING.md#storybook)** + \- used for component stories and base UI testing. + +* **[Feature flags](https://github.com/flyteorg/flyteconsole/blob/narusina/fflags/CONTRIBUTING.md#feature-flags)** + \- allows to enable/disable specific code paths. Used to simplify continious development. + +* **[Google Analytics](https://github.com/flyteorg/flyteconsole/blob/narusina/fflags/CONTRIBUTING.md#google-analytics)** + \- adds tracking code to the app or website. To disable use `ENABLE_GA=false` + +More info on each section could be found at [CONTRIBUTING.md](CONTRIBUTING.md) ### CORS Proxying: Recommended setup diff --git a/src/basics/FeatureFlags/FeatureFlags.test.tsx b/src/basics/FeatureFlags/FeatureFlags.test.tsx new file mode 100644 index 0000000000..4385aab4ed --- /dev/null +++ b/src/basics/FeatureFlags/FeatureFlags.test.tsx @@ -0,0 +1,69 @@ +import * as React from 'react'; +import { render, screen, waitFor } from '@testing-library/react'; + +import { FeatureFlagsProvider, useFeatureFlag } from '.'; +import { FeatureFlag } from './defaultConfig'; + +function TestContent() { + const enabledTestFlag = useFeatureFlag(FeatureFlag.TestFlagUndefined); + return ( + + + + ); +} + +function TestPage() { + return ( + + + + ); +} + +declare global { + interface Window { + setFeatureFlag: (flag: FeatureFlag, newValue: boolean) => void; + getFeatureFlag: (flag: FeatureFlag) => boolean; + clearRuntimeConfig: () => void; + } +} + +describe('FeatureFlags', () => { + beforeEach(() => { + render(); + }); + + afterEach(() => { + window.clearRuntimeConfig(); + }); + + it('Feature flags can be read/set from dev tools', async () => { + // flag defined and return proper value + expect(window.getFeatureFlag(FeatureFlag.TestFlagTrue)).toBeTruthy(); + // flag undefined and returns false + expect( + window.getFeatureFlag(FeatureFlag.TestFlagUndefined) + ).toBeFalsy(); + + window.setFeatureFlag(FeatureFlag.TestFlagUndefined, true); + await waitFor(() => { + // check that flag cghanged value + expect( + window.getFeatureFlag(FeatureFlag.TestFlagUndefined) + ).toBeTruthy(); + }); + }); + + it('useFeatureFlags returns proper live value', async () => { + // default value - flag is disabled + expect(screen.getByText(/Disabled/i)).toBeTruthy(); + + // Enable flag + window.setFeatureFlag(FeatureFlag.TestFlagUndefined, true); + await waitFor(() => { + // check that component was updated accordingly + expect(screen.getByText(/Enabled/i)).toBeTruthy(); + }); + }); +}); diff --git a/src/basics/FeatureFlags/defaultConfig.ts b/src/basics/FeatureFlags/defaultConfig.ts new file mode 100644 index 0000000000..f7a861320d --- /dev/null +++ b/src/basics/FeatureFlags/defaultConfig.ts @@ -0,0 +1,23 @@ +/** + * Default Feature Flag config - used for features in developement. + */ + +export enum FeatureFlag { + // Test flag is created only for unit-tests + TestFlagUndefined = 'test-flag-undefined', + TestFlagTrue = 'test-flag-true', + + // Production flags + LaunchPlan = 'launch-plan' +} + +export type FeatureFlagConfig = { [k: string]: boolean }; + +export const defaultFlagConfig: FeatureFlagConfig = { + // Test + 'test-flag-true': true, + + // Production - new code should be turned off by default + // If you need to turn it on locally -> update runtimeConfig in ./index.tsx file + 'launch-plan': false +}; diff --git a/src/basics/FeatureFlags/index.tsx b/src/basics/FeatureFlags/index.tsx new file mode 100644 index 0000000000..e7814e6c29 --- /dev/null +++ b/src/basics/FeatureFlags/index.tsx @@ -0,0 +1,102 @@ +/** + * Feature Flag provider - allows a multi-stage development. + */ +import * as React from 'react'; +import { + createContext, + useCallback, + useContext, + useEffect, + useState +} from 'react'; +import { isDevEnv, isTestEnv } from 'common/env'; +import { + defaultFlagConfig, + FeatureFlag, + FeatureFlagConfig +} from './defaultConfig'; + +export { FeatureFlag } from './defaultConfig'; + +// To turn on flag for local development only - update flag value here +// REMOVE change prior to commit +let runtimeConfig: FeatureFlagConfig = { + ...defaultFlagConfig + // 'test-flag-true': true, <== locally turns flag on +}; + +interface FeatureFlagState { + flags: FeatureFlagConfig; + setFeatureFlag: (flag: FeatureFlag, newValue: boolean) => void; + getFeatureFlag: (flag: FeatureFlag) => boolean; +} + +interface FeatureFlagProviderProps { + children?: React.ReactNode; +} + +/** FeatureFlagContext - used only if ContextProvider wasn't initialized */ +const FeatureFlagContext = createContext({ + flags: defaultFlagConfig, + setFeatureFlag: () => { + /* Provider is not initialized */ + }, + getFeatureFlag: () => false +}); + +/** useFeatureFlag - should be used to get flag value */ +export const useFeatureFlag = (flag: FeatureFlag) => + useContext(FeatureFlagContext).getFeatureFlag(flag); + +/** useFatureFlagContext - could be used to set flags from code */ +export const useFatureFlagContext = () => useContext(FeatureFlagContext); + +/** FeatureFlagsProvider - should wrap top level component for Production or feature flag related testing*/ +export const FeatureFlagsProvider = (props: FeatureFlagProviderProps) => { + const [flags, setFlags] = useState({ + ...defaultFlagConfig, + ...runtimeConfig + }); + + const setFeatureFlag = useCallback( + (flag: FeatureFlag, newValue: boolean) => { + runtimeConfig[flag] = newValue; + setFlags({ ...defaultFlagConfig, ...runtimeConfig }); + }, + [] + ); + + const getFeatureFlag = useCallback( + (flag: FeatureFlag) => { + if (isDevEnv() && flags[flag] === undefined) { + throw `Default config value is absent for ${flag}`; + } + return flags[flag] ?? false; + }, + [flags] + ); + + const clearRuntimeConfig = useCallback(() => { + runtimeConfig = { ...defaultFlagConfig }; + }, []); + + useEffect(() => { + if (isDevEnv() || isTestEnv()) { + // Allow manual change of feature flags from devtools + window['setFeatureFlag'] = setFeatureFlag; + window['getFeatureFlag'] = getFeatureFlag; + if (isTestEnv()) { + // allow reset flags to default - should be used in testing environment only + window['clearRuntimeConfig'] = clearRuntimeConfig; + } + } + }, [setFeatureFlag, getFeatureFlag, clearRuntimeConfig]); + + return ( + + {props.children} + + ); +}; diff --git a/src/common/env.ts b/src/common/env.ts index 21e80c0761..b73a2c504d 100644 --- a/src/common/env.ts +++ b/src/common/env.ts @@ -3,3 +3,6 @@ import { Env } from 'config/types'; /** equivalent to process.env in server and client */ // tslint:disable-next-line:no-any export const env: Env = Object.assign({}, process.env, window.env); + +export const isDevEnv = () => env.NODE_ENV === 'development'; +export const isTestEnv = () => env.NODE_ENV === 'test'; diff --git a/src/components/App/App.tsx b/src/components/App/App.tsx index 5e5151bf74..e5b205119d 100644 --- a/src/components/App/App.tsx +++ b/src/components/App/App.tsx @@ -1,5 +1,6 @@ import { CssBaseline } from '@material-ui/core'; import { ThemeProvider } from '@material-ui/styles'; +import { FeatureFlagsProvider } from 'basics/FeatureFlags'; import { env } from 'common/env'; import { debug, debugPrefix } from 'common/log'; import { ErrorBoundary } from 'components/common/ErrorBoundary'; @@ -32,34 +33,36 @@ export const AppComponent: React.FC = () => { const apiState = useAPIState(); return ( - - - - - - - - Flyte Console - - - - - - - - - - - - - - + + + + + + + + + Flyte Console + + + + + + + + + + + + + + + ); }; diff --git a/src/components/Executions/Tables/styles.ts b/src/components/Executions/Tables/styles.ts index 14136ea994..2bda3d57cf 100644 --- a/src/components/Executions/Tables/styles.ts +++ b/src/components/Executions/Tables/styles.ts @@ -194,11 +194,14 @@ export const useWorkflowExecutionsColumnStyles = makeStyles((theme: Theme) => ({ marginLeft: theme.spacing(2), marginRight: theme.spacing(2), textAlign: 'left' + }, + rightMargin: { + marginRight: theme.spacing(1) } })); /** Style overrides specific to columns in `WorkflowVersionsTable`. */ -export const useWorkflowVersionsColumnStyles = makeStyles((theme: Theme) => ({ +export const useWorkflowVersionsColumnStyles = makeStyles(() => ({ columnRadioButton: { width: workflowVersionsTableColumnWidths.radio }, diff --git a/src/components/Executions/Tables/useWorkflowExecutionsTableColumns.tsx b/src/components/Executions/Tables/useWorkflowExecutionsTableColumns.tsx index 0e139fc547..08f3a758f0 100644 --- a/src/components/Executions/Tables/useWorkflowExecutionsTableColumns.tsx +++ b/src/components/Executions/Tables/useWorkflowExecutionsTableColumns.tsx @@ -1,4 +1,6 @@ -import { Link, Typography } from '@material-ui/core'; +import { IconButton, Typography } from '@material-ui/core'; +import InputOutputIcon from '@material-ui/icons/Tv'; +import LaunchPlanIcon from '@material-ui/icons/AssignmentOutlined'; import { dateFromNow, formatDateLocalTimezone, @@ -14,10 +16,12 @@ import { getWorkflowExecutionTimingMS } from '../utils'; import { useWorkflowExecutionsColumnStyles } from './styles'; import { WorkflowExecutionColumnDefinition } from './types'; import { WorkflowExecutionLink } from './WorkflowExecutionLink'; +import { FeatureFlag, useFeatureFlag } from 'basics/FeatureFlags'; interface WorkflowExecutionColumnOptions { showWorkflowName: boolean; } + /** Returns a memoized list of column definitions to use when rendering a * `WorkflowExecutionRow`. Memoization is based on common/column style objects * and any fields in the incoming `WorkflowExecutionColumnOptions` object. @@ -27,6 +31,8 @@ export function useWorkflowExecutionsTableColumns({ }: WorkflowExecutionColumnOptions): WorkflowExecutionColumnDefinition[] { const styles = useWorkflowExecutionsColumnStyles(); const commonStyles = useCommonStyles(); + const isLaunchPlanEnabled = useFeatureFlag(FeatureFlag.LaunchPlan); + return React.useMemo( () => [ { @@ -112,13 +118,27 @@ export function useWorkflowExecutionsTableColumns({ const onClick = () => state.setSelectedIOExecution(execution); return ( - - View Inputs & Outputs - + <> + + + + {isLaunchPlanEnabled && ( + { + /* Not implemented */ + }} + > + + + )} + ); }, className: styles.columnInputsOutputs, @@ -126,6 +146,6 @@ export function useWorkflowExecutionsTableColumns({ label: '' } ], - [styles, commonStyles, showWorkflowName] + [styles, commonStyles, showWorkflowName, isLaunchPlanEnabled] ); } diff --git a/src/components/Theme/muiTheme.ts b/src/components/Theme/muiTheme.ts index ce47a7d863..ee1b833562 100644 --- a/src/components/Theme/muiTheme.ts +++ b/src/components/Theme/muiTheme.ts @@ -33,7 +33,7 @@ const theme = createMuiTheme({ }, action: { selected: selectedActionColor, - hoverOpacity: 0.3 + hoverOpacity: 0.15 } }, props: { diff --git a/src/config/types.ts b/src/config/types.ts index 63778623a8..a642cdaf99 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -3,6 +3,6 @@ export interface Env extends NodeJS.ProcessEnv { BASE_URL?: string; CORS_PROXY_PREFIX: string; DISABLE_ANALYTICS?: string; - NODE_ENV?: 'development' | 'production'; + NODE_ENV?: 'development' | 'production' | 'test'; STATUS_URL?: string; }