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

[KED-824][KED-788] Use new JSON API in app, and calculate transitive links on the fly #8

Merged
merged 18 commits into from
Jul 15, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
# CSS
/src/**/*.css

# JSON viz data (generated from logs)
# JSON viz data
/public/data
/public/logs
/public/api

# dependencies
/node_modules
Expand Down
43 changes: 25 additions & 18 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,36 +33,42 @@ Typically, small contributions to Kedro-Viz are more preferable due to an easier
## Your first contribution

Working on your first pull request? You can learn how from these resources:
* [First timers only](https://www.firsttimersonly.com/)
* [How to contribute to an open source project on GitHub](https://egghead.io/courses/how-to-contribute-to-an-open-source-project-on-github)

- [First timers only](https://www.firsttimersonly.com/)
- [How to contribute to an open source project on GitHub](https://egghead.io/courses/how-to-contribute-to-an-open-source-project-on-github)

### Guidelines
> *Note:* We only accept contributions under the [Apache 2.0](https://opensource.org/licenses/Apache-2.0) license and you should have permission to share the submitted code.

* Aim for cross-platform compatibility on Windows, macOS and Linux, and support recent versions of major browsers
* We use [SemVer](https://semver.org/) for versioning
* Our code is designed to be compatible with Python 3.5 onwards
* We use [Anaconda](https://www.anaconda.com/distribution/) as a preferred virtual environment
* We use [PEP 8 conventions](https://www.python.org/dev/peps/pep-0008/) for all Python code
* We use [Google docstrings](https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings) for code comments
* We use [PEP 484 type hints](https://www.python.org/dev/peps/pep-0484/) for all user-facing functions / class methods e.g.

> _Note:_ We only accept contributions under the [Apache 2.0](https://opensource.org/licenses/Apache-2.0) license and you should have permission to share the submitted code.

- Aim for cross-platform compatibility on Windows, macOS and Linux, and support recent versions of major browsers
- We use [SemVer](https://semver.org/) for versioning
- Our code is designed to be compatible with Python 3.5 onwards
- We use [Anaconda](https://www.anaconda.com/distribution/) as a preferred virtual environment
- We use [PEP 8 conventions](https://www.python.org/dev/peps/pep-0008/) for all Python code
- We use [Google docstrings](https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings) for code comments
- We use [PEP 484 type hints](https://www.python.org/dev/peps/pep-0484/) for all user-facing functions / class methods e.g.

```
def count_truthy(elements: List[Any]) -> int:
return sum(1 for elem in elements if elem)
```
* Keep UI elements fully keyboard accessible, and aim to support screen-readers where possible
* Maintain a high level of animation performance, and minimise page-load time

- Keep UI elements fully keyboard accessible, and aim to support screen-readers where possible
- Maintain a high level of animation performance, and minimise page-load time

### JavaScript Development

First, clone this repo. Then install dependencies (`npm i`). Now you're ready to begin development.

If you want to use a particular dataset, you'll first need to place it in `/public/logs/nodes.json`. Otherwise, you can serve randomly-generated data.

Kedro-Viz uses an environment variable to configure the data source. You can set it when starting up the dev server, e.g. `DATA=random npm start` will serve random procedurally-generated data (refreshed on each page-load). When random data is enabled, certain other features like snapshot history are also enabled by default to help with browser testing. This is usually the most useful setting for local development.
Kedro-Viz uses an environment variable to configure the data source. You can set it when starting up the dev server, e.g. `DATA=random npm start` will serve random procedurally-generated data (refreshed on each page-load). When random or mock data is enabled, certain other features like snapshot history are also enabled by default to help with browser testing. This is usually the most useful setting for local development.

In other words, to run the app in development mode on a local server, use one of the following:

- `DATA=random npm start` --> Serve randomly-generated data
- `DATA=mock npm start` --> Serve example test data, from `/src/utils/data.mock.js`
- `npm start` --> Serve data loaded from `/public/logs/nodes.json`

This will serve the app at [localhost:4141](http://localhost:4141/), and watch files in `/src` for changes. It will also update the `/lib` directory, which contains a Babel-compiled copy of the source. This directory is exported to `npm`, and is used when importing as a React component into another application. It is updated automatically on save in case you need to test/debug it locally (e.g. with `npm link`). You can also update it manually, by running
Expand All @@ -84,10 +90,10 @@ We use a branching model that helps us keep track of branches in a logical, cons

## Plugin contribution process

1. Fork the project
2. Develop your contribution in a new branch and open a PR against the `develop` branch
3. Make sure the CI builds are green (have a look at the section [Running checks locally](#running-checks-locally) below)
4. Update the PR according to the reviewer's comments
1. Fork the project
2. Develop your contribution in a new branch and open a PR against the `develop` branch
3. Make sure the CI builds are green (have a look at the section [Running checks locally](#running-checks-locally) below)
4. Update the PR according to the reviewer's comments

### JavaScript application tests

Expand All @@ -112,6 +118,7 @@ npm run test:coverage
See the [Create-React-App docs](https://github.com/facebook/create-react-app) for further information on JS testing.

## Python web server tests

To run E2E tests you need to install the test requirements which includes `behave`, do this using the following command:

```bash
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ As a JavaScript React component, the project is designed to be used in two diffe

1. **Standalone application**

Run `npm run build` to generate a production build as a full-page app. The built app will be placed in the `/build` directory. Data for the chart should be placed in `/public/logs/nodes.json` because this directory is marked `gitignore`.
Run `npm run build` to generate a production build as a full-page app. The built app will be placed in the `/build` directory. Data for the chart should be placed in `/public/api/nodes.json` because this directory is marked `gitignore`.

2. **React component**

Expand Down
2 changes: 1 addition & 1 deletion src/actions/actions.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { mockData } from '../utils/data.mock';
import mockData from '../utils/data.mock';
import {
CHANGE_ACTIVE_SNAPSHOT,
CHANGE_VIEW,
Expand Down
12 changes: 8 additions & 4 deletions src/components/app/app.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { shallow } from 'enzyme';
import App from './index';
import { mockData } from '../../utils/data.mock';
import mockData from '../../utils/data.mock';

describe('App', () => {
describe('renders without crashing', () => {
Expand Down Expand Up @@ -30,14 +30,18 @@ describe('App', () => {

it('when data prop is set on first load', () => {
const wrapper = shallow(<App data={mockData} />);
expect(getSnapshotIDs(wrapper)).toHaveLength(mockData.length);
expect(getSnapshotIDs(wrapper)).toHaveLength(mockData.snapshots.length);
});

it('when data prop is updated', () => {
const wrapper = shallow(<App data={mockData} />);
const newMockData = Object.assign([], mockData.concat(mockData[0]));
const newMockData = Object.assign({}, mockData, {
snapshots: [...mockData.snapshots, mockData.snapshots[0]]
});
wrapper.setProps({ data: newMockData });
expect(getSnapshotIDs(wrapper)).toHaveLength(newMockData.length);
expect(getSnapshotIDs(wrapper)).toHaveLength(
newMockData.snapshots.length
);
});
});

Expand Down
21 changes: 12 additions & 9 deletions src/components/app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class App extends React.Component {
*/
dataWasUpdated(prevData, newData) {
// Check just the schema IDs of incoming data updates
const dataID = snapshots =>
const dataID = ({ snapshots }) =>
Array.isArray(snapshots) && snapshots.map(d => d.schema_id).join('');

return dataID(prevData) !== dataID(newData);
Expand All @@ -60,14 +60,17 @@ App.propTypes = {
allowHistoryDeletion: PropTypes.bool,
data: PropTypes.oneOfType([
PropTypes.string,
PropTypes.arrayOf(
PropTypes.shape({
created_ts: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
json_schema: PropTypes.array.isRequired,
schema_id: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
message: PropTypes.string
})
)
PropTypes.shape({
snapshots: PropTypes.arrayOf(
PropTypes.shape({
created_ts: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),
edges: PropTypes.array.isRequired,
message: PropTypes.string,
nodes: PropTypes.array.isRequired,
tags: PropTypes.array
})
)
})
]),
onDeleteSnapshot: PropTypes.func,
showHistory: PropTypes.bool
Expand Down
9 changes: 6 additions & 3 deletions src/components/app/load-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { json } from 'd3-fetch';
import config from '../../config';
import getRandomHistory from '../../utils/random-data';
import formatSnapshots from '../../utils/format-data';
import mockData from '../../utils/data.mock';
import { loadState } from '../../utils';

/**
Expand Down Expand Up @@ -48,9 +49,11 @@ export const loadData = (data, onLoadData) => {
switch (data) {
case 'random':
return formatSnapshots(getRandomHistory());
case 'mock':
return formatSnapshots(mockData);
case 'json':
loadJsonData().then(onLoadData);
return formatSnapshots([]);
return formatSnapshots({ snapshots: [] });
case null:
throw new Error('No data was provided to App component via props');
default:
Expand All @@ -64,10 +67,10 @@ export const loadData = (data, onLoadData) => {
export const loadJsonData = () => {
const { dataPath } = config();
return json(dataPath)
.then(json_schema => formatSnapshots([{ json_schema }]))
.catch(() => {
throw new Error(
`Unable to load pipeline data. Please check that you have placed a file at ${dataPath}`
);
});
})
.then(formatSnapshots);
};
2 changes: 1 addition & 1 deletion src/components/chart-ui/chart-ui.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import ChartUI, {
mapStateToProps,
mapDispatchToProps
} from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';

describe('ChartUI', () => {
it('renders without crashing', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/description/description.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Description, mapStateToProps } from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';
import {
getActiveSnapshotMessage,
getActiveSnapshotTimestamp
Expand Down
2 changes: 1 addition & 1 deletion src/components/flowchart/flowchart.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import $ from 'cheerio';
import FlowChart, { mapStateToProps, mapDispatchToProps } from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';
import { getActiveSnapshotNodes } from '../../selectors/nodes';

describe('FlowChart', () => {
Expand Down
16 changes: 8 additions & 8 deletions src/components/flowchart/linked-nodes.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,21 @@ export const getLinkedNodes = (edges, nodeID) => {
const linkedNodes = [];

(function getParents(id) {
edges
.filter(d => d.target === id)
.forEach(d => {
edges.forEach(d => {
if (d.target === id) {
linkedNodes.push(d.source);
getParents(d.source);
});
}
});
})(nodeID);

(function getChildren(id) {
edges
.filter(d => d.source === id)
.forEach(d => {
edges.forEach(d => {
if (d.source === id) {
linkedNodes.push(d.target);
getChildren(d.target);
});
}
});
})(nodeID);

return linkedNodes;
Expand Down
2 changes: 1 addition & 1 deletion src/components/flowchart/linked-nodes.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { getLinkedNodes } from './linked-nodes';
import { getLayout } from '../../selectors/layout';
import { mockState } from '../../utils/data.mock';
import { mockState } from '../../utils/state.mock';

describe('getLinkedNodes function', () => {
it('should search through edges for ancestor and descendant nodes', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/history/history.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import History, {
mapStateToProps,
mapDispatchToProps
} from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';
import { getSnapshotHistory } from '../../selectors';

const props = {
Expand Down
2 changes: 1 addition & 1 deletion src/components/node-list/node-list.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
}

.pipeline-node-list-scrollbars {
min-height: 150px;
min-height: 400px;

.kui-theme--light & {
background: $color-bg-light-alt;
Expand Down
2 changes: 1 addition & 1 deletion src/components/node-list/node-list.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import NodeList, { mapStateToProps, mapDispatchToProps } from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';
import { getNodes } from '../../selectors/nodes';

describe('NodeList', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/sidebar-tabs/sidebar-tabs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import SidebarTabs, {
SidebarTabs as UnconnectedSidebarTabs,
mapStateToProps
} from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';

describe('SidebarTabs', () => {
it('renders without crashing', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/tag-list/tag-list.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import TagList, {
mapStateToProps,
mapDispatchToProps
} from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';

describe('TagList', () => {
it('renders without crashing', () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/wrapper/wrapper.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Wrapper, mapStateToProps } from './index';
import { mockState, setup } from '../../utils/data.mock';
import { mockState, setup } from '../../utils/state.mock';

const { showHistory, theme } = mockState;
const mockProps = { showHistory, theme };
Expand Down
2 changes: 1 addition & 1 deletion src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Generate a configuration object for use across the application
*/
const config = () => ({
dataPath: './logs/nodes.json',
dataPath: './api/nodes.json',
dataSource: process.env.REACT_APP_DATA_SOURCE || 'json',
localStorageName: 'KedroViz'
});
Expand Down
6 changes: 3 additions & 3 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import config from './config';
import './styles/index.css';

const { dataSource } = config();
const useRandomData = dataSource === 'random';
const showHistory = dataSource === 'random' || dataSource === 'mock';

ReactDOM.render(
<App
allowHistoryDeletion={useRandomData}
allowHistoryDeletion={showHistory}
data={dataSource}
showHistory={useRandomData}
showHistory={showHistory}
/>,
document.getElementById('root')
);
4 changes: 2 additions & 2 deletions src/reducers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ function reducer(state = {}, action) {
}

case TOGGLE_PARAMETERS: {
const paramIDs = state.snapshotNodes[state.activeSnapshot].filter(id =>
state.nodeName[id].includes('param')
const paramIDs = state.snapshotNodes[state.activeSnapshot].filter(
id => state.nodeIsParam[id]
);
return Object.assign({}, state, {
nodeDisabled: paramIDs.reduce(
Expand Down
Loading