Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate visualizations into their own package #4837

Merged
merged 21 commits into from
May 6, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,12 @@ jobs:
- run: sudo pip3 install -r requirements_bundles.txt
- run: npm ci
- run: npm run bundle
- run: npm test
- run:
name: Run App Tests
command: npm test
- run:
name: Run Visualizations Tests
command: (cd viz-lib && npm test)
- run: npm run lint
frontend-e2e-tests:
environment:
Expand Down
1 change: 1 addition & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
client/.tmp/
client/dist/
node_modules/
viz-lib/node_modules/
.tmp/
.venv/
venv/
Expand Down
3 changes: 2 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ FROM node:12 as frontend-builder

WORKDIR /frontend
COPY package.json package-lock.json /frontend/
RUN npm ci
COPY viz-lib /frontend/viz-lib
RUN npm ci --unsafe-perm
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be required to run the preinstall script (which installs viz-lib deps and build it) when we run the command as root user. An alternative is to switch the user to the redash one in this scope. I tried to switch to it for all commands, but didn't seem that quick. Another option is to switch to the user only while running this command.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but didn't seem that quick

in terms of effort required to make the switch?

Regardless, --unsafe-perm should be safe in the context of running inside a Docker container build...

Copy link
Member Author

@gabrieldutra gabrieldutra Apr 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in terms of effort required to make the switch?

In terms of changes in that file, mostly needs to update chown/chmod permissions for that folder. As the --unsafe-perm seemed simpler, I wanted to show it as an option, but needed to confirm regarding safety. I actually didn't sit down and actually tried to change thos yesterday, but I did today and I could get it to work like this:

FROM node:12 as frontend-builder

RUN useradd --create-home redash

WORKDIR /frontend

COPY package.json package-lock.json webpack.config.js /frontend/
COPY viz-lib /frontend/viz-lib
COPY client /frontend/client

RUN chown -R redash /frontend
USER redash
RUN npm ci
RUN npm run build

(notice I changed the COPY order to run only one chown, not sure if there was a reason to have it after the npm ci command though)

Anyway, LMK which option you prefer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(notice I changed the COPY order to run only one chown, not sure if there was a reason to have it after the npm ci command though)

The reason for the current order is to make sure that we don't have to reinstall if package.json/package-lock.json didn't change, as they change less frequently than the codebase.

Let's go with the unsafe option for now.


COPY client /frontend/client
COPY webpack.config.js /frontend/
Expand Down
2 changes: 1 addition & 1 deletion client/app/components/ApplicationArea/Router.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { isFunction, startsWith, trimStart, trimEnd } from "lodash";
import React, { useState, useEffect, useRef } from "react";
import PropTypes from "prop-types";
import UniversalRouter from "universal-router";
import ErrorBoundary from "@/components/ErrorBoundary";
import ErrorBoundary from "@redash/viz/lib/components/ErrorBoundary";
import location from "@/services/location";
import url from "@/services/url";

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { useEffect, useState, useContext } from "react";
import PropTypes from "prop-types";
import { ErrorBoundaryContext } from "@/components/ErrorBoundary";
import { ErrorBoundaryContext } from "@redash/viz/lib/components/ErrorBoundary";
import { Auth } from "@/services/auth";

// This wrapper modifies `route.render` function and instead of passing `currentRoute` passes an object
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { useEffect, useState } from "react";
import PropTypes from "prop-types";
import ErrorBoundary, { ErrorBoundaryContext } from "@/components/ErrorBoundary";
import ErrorBoundary, { ErrorBoundaryContext } from "@redash/viz/lib/components/ErrorBoundary";
import { Auth } from "@/services/auth";
import organizationStatus from "@/services/organizationStatus";
import ApplicationHeader from "./ApplicationHeader";
Expand Down
2 changes: 1 addition & 1 deletion client/app/components/Parameters.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { size, filter, forEach, extend } from "lodash";
import React from "react";
import PropTypes from "prop-types";
import { SortableContainer, SortableElement, DragHandle } from "@/components/sortable";
import { SortableContainer, SortableElement, DragHandle } from "@redash/viz/lib/components/sortable";
import location from "@/services/location";
import { Parameter, createParameter } from "@/services/parameters";
import ParameterApplyButton from "@/components/ParameterApplyButton";
Expand Down
2 changes: 1 addition & 1 deletion client/app/components/QueryLink.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from "react";
import PropTypes from "prop-types";
import { VisualizationType } from "@/visualizations/prop-types";
import { VisualizationType } from "@redash/viz/lib";
import VisualizationName from "@/components/visualizations/VisualizationName";

import "./QueryLink.less";
Expand Down
2 changes: 1 addition & 1 deletion client/app/components/dashboards/TextboxDialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import Modal from "antd/lib/modal";
import Input from "antd/lib/input";
import Tooltip from "antd/lib/tooltip";
import Divider from "antd/lib/divider";
import HtmlContent from "@/components/HtmlContent";
import HtmlContent from "@redash/viz/lib/components/HtmlContent";
import { wrap as wrapDialog, DialogPropType } from "@/components/DialogWrapper";
import notification from "@/services/notification";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { useState } from "react";
import PropTypes from "prop-types";
import { markdown } from "markdown";
import Menu from "antd/lib/menu";
import HtmlContent from "@/components/HtmlContent";
import HtmlContent from "@redash/viz/lib/components/HtmlContent";
import TextboxDialog from "@/components/dashboards/TextboxDialog";
import Widget from "./Widget";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import { compact, isEmpty, invoke } from "lodash";
import { markdown } from "markdown";
import cx from "classnames";
import Menu from "antd/lib/menu";
import HtmlContent from "@redash/viz/lib/components/HtmlContent";
import { currentUser } from "@/services/auth";
import recordEvent from "@/services/recordEvent";
import { formatDateTime } from "@/lib/utils";
import HtmlContent from "@/components/HtmlContent";
import Parameters from "@/components/Parameters";
import TimeAgo from "@/components/TimeAgo";
import Timer from "@/components/Timer";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ import notification from "@/services/notification";
import Visualization from "@/services/visualization";
import recordEvent from "@/services/recordEvent";
import getQueryResultData from "@/lib/getQueryResultData";
import { VisualizationType } from "@/visualizations/prop-types";
import { Renderer, Editor } from "@/components/visualizations/visualizationComponents";
import registeredVisualizations, {
import {
registeredVisualizations,
getDefaultVisualization,
newVisualization,
} from "@/visualizations/registeredVisualizations";
VisualizationType,
} from "@redash/viz/lib";
import { Renderer, Editor } from "@/components/visualizations/visualizationComponents";

import "./EditVisualizationDialog.less";

Expand Down
3 changes: 1 addition & 2 deletions client/app/components/visualizations/VisualizationName.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React from "react";
import { VisualizationType } from "@/visualizations/prop-types";
import registeredVisualizations from "@/visualizations/registeredVisualizations";
import { VisualizationType, registeredVisualizations } from "@redash/viz/lib";

import "./VisualizationName.less";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import PropTypes from "prop-types";
import getQueryResultData from "@/lib/getQueryResultData";
import { getColumnCleanName } from "@/services/query-result";
import Filters, { FiltersType, filterData } from "@/components/Filters";
import { VisualizationType } from "@/visualizations/prop-types";
import { VisualizationType } from "@redash/viz/lib";
import { Renderer } from "@/components/visualizations/visualizationComponents";

function combineFilters(localFilters, globalFilters) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import React from "react";
import { pick } from "lodash";
import HelpTrigger from "@/components/HelpTrigger";
import { Renderer as VisRenderer, Editor as VisEditor } from "@/visualizations";
import { updateVisualizationsSettings } from "@/visualizations/visualizationsSettings";
import { Renderer as VisRenderer, Editor as VisEditor, updateVisualizationsSettings } from "@redash/viz/lib";
import { clientConfig } from "@/services/auth";

import countriesDataUrl from "@/visualizations/choropleth/maps/countries.geo.json";
import subdivJapanDataUrl from "@/visualizations/choropleth/maps/japan.prefectures.geo.json";
import countriesDataUrl from "@redash/viz/lib/visualizations/choropleth/maps/countries.geo.json";
import subdivJapanDataUrl from "@redash/viz/lib/visualizations/choropleth/maps/japan.prefectures.geo.json";

function wrapComponentWithSettings(WrappedComponent) {
return function VisualizationComponent(props) {
Expand Down
2 changes: 1 addition & 1 deletion client/app/config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import url from "@/services/url";
import "@/assets/images/avatar.svg";

// Register visualizations
import "@/visualizations";
import "@redash/viz/lib";

import "./antd-spinner";

Expand Down
4 changes: 2 additions & 2 deletions client/app/pages/queries/VisualizationEmbed.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,19 @@ import Dropdown from "antd/lib/dropdown";
import Icon from "antd/lib/icon";
import Menu from "antd/lib/menu";
import Tooltip from "antd/lib/tooltip";
import HtmlContent from "@redash/viz/lib/components/HtmlContent";
import routeWithApiKeySession from "@/components/ApplicationArea/routeWithApiKeySession";
import { Query } from "@/services/query";
import location from "@/services/location";
import { formatDateTime } from "@/lib/utils";
import HtmlContent from "@/components/HtmlContent";
import Parameters from "@/components/Parameters";
import { Moment } from "@/components/proptypes";
import TimeAgo from "@/components/TimeAgo";
import Timer from "@/components/Timer";
import QueryResultsLink from "@/components/EditVisualizationButton/QueryResultsLink";
import VisualizationName from "@/components/visualizations/VisualizationName";
import VisualizationRenderer from "@/components/visualizations/VisualizationRenderer";
import { VisualizationType } from "@/visualizations/prop-types";
import { VisualizationType } from "@redash/viz/lib";
import logoUrl from "@/assets/images/redash_icon_small.png";

function VisualizationEmbedHeader({ queryName, queryDescription, visualization }) {
Expand Down
2 changes: 1 addition & 1 deletion client/app/services/widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { each, pick, extend, isObject, truncate, keys, difference, filter, map,
import location from "@/services/location";
import { cloneParameter } from "@/services/parameters";
import dashboardGridOptions from "@/config/dashboard-grid-options";
import registeredVisualizations from "@/visualizations/registeredVisualizations";
import { registeredVisualizations } from "@redash/viz/lib";
import { Query } from "./query";

export const WidgetTypeEnum = {
Expand Down
Loading