Skip to content

Commit

Permalink
fix(Deployment): refactor Deployment fields to fix perf issues
Browse files Browse the repository at this point in the history
This corresponds to changes in ibi-group/datatools-server#310

fix #574
  • Loading branch information
landonreed committed May 7, 2020
1 parent af00ed8 commit 9b66f89
Show file tree
Hide file tree
Showing 14 changed files with 79 additions and 45 deletions.
2 changes: 1 addition & 1 deletion __tests__/test-utils/mock-data/manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export function makeMockDeployment (
osmExtractUrl: null,
otpCommit: null,
otpVersion: null,
project,
projectId: project.id,
projectBounds: {east: 0, west: 0, north: 0, south: 0},
r5: false,
r5Version: null,
Expand Down
10 changes: 5 additions & 5 deletions lib/manager/actions/deployments.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export function deployToTarget (deployment: Deployment, target: string) {
} else {
// If the deployment request succeeded, start monitoring the job.
dispatch(startJobMonitor())
dispatch(fetchProjectDeployments(deployment.project.id))
dispatch(fetchProjectDeployments(deployment.projectId))
}
})
.then(json => {
Expand Down Expand Up @@ -155,12 +155,12 @@ export function fetchDeploymentAndProject (id: string) {

export function deleteDeployment (deployment: Deployment & { isCreating?: boolean }) {
return function (dispatch: dispatchFn, getState: getStateFn) {
const {id, isCreating, project} = deployment
const {id, isCreating, projectId} = deployment
if (isCreating) {
return dispatch(fetchProjectDeployments(project.id))
return dispatch(fetchProjectDeployments(projectId))
}
return dispatch(secureFetch(`${DEPLOYMENT_URL}/${id}`, 'delete'))
.then((res) => dispatch(fetchProjectDeployments(project.id)))
.then((res) => dispatch(fetchProjectDeployments(projectId)))
}
}

Expand Down Expand Up @@ -191,7 +191,7 @@ export function saveDeployment (
dispatch(savingDeployment())
return dispatch(secureFetch(DEPLOYMENT_URL, 'post', props))
.then(response => response.json())
.then(deployment => dispatch(fetchProjectDeployments(deployment.project.id)))
.then(deployment => dispatch(fetchProjectDeployments(deployment.projectId)))
}
}

Expand Down
38 changes: 29 additions & 9 deletions lib/manager/components/CurrentDeploymentPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,21 @@ import { getActiveInstanceCount, getServerForId } from '../util/deployment'
import DeploymentPreviewButton from './DeploymentPreviewButton'
import EC2InstanceCard from '../../common/components/EC2InstanceCard'

import type { Deployment, DeploySummary, EC2InstanceSummary, OtpServer, ServerJob } from '../../types'
import type {
Deployment,
DeploySummary,
EC2InstanceSummary,
OtpServer,
Project,
ServerJob
} from '../../types'

type Props = {
deployJobs: Array<ServerJob>,
deployment: Deployment,
downloadBuildArtifact: typeof deploymentActions.downloadBuildArtifact,
fetchDeployment: typeof deploymentActions.fetchDeployment,
project: Project,
server: ?OtpServer,
terminateEC2InstanceForDeployment: typeof deploymentActions.terminateEC2InstanceForDeployment
}
Expand All @@ -35,6 +43,12 @@ export default class CurrentDeploymentPanel extends Component<Props, State> {
activeSummaryIndex: 0
}

componentDidMount () {
// Fetch single deployment. This JSON response will contain the #ec2Instances
// field, which does not come with this field (to prevent a very slow response).
this.props.fetchDeployment(this.props.deployment.id)
}

componentWillReceiveProps (nextProps: Props) {
// If status message changes while deployment job is in progress, re-fetch
// deployment to update EC2 instance status.
Expand All @@ -55,7 +69,7 @@ export default class CurrentDeploymentPanel extends Component<Props, State> {
if (this.props.server) return this.props.server
const deployJob = this._getDeployJob()
const serverId = deployJob && deployJob.serverId
return getServerForId(serverId, this.props.deployment.project)
return getServerForId(serverId, this.props.project)
}

handleDecrementSummary = () => {
Expand Down Expand Up @@ -84,11 +98,12 @@ export default class CurrentDeploymentPanel extends Component<Props, State> {
}

render () {
const { deployJobs, deployment } = this.props
const { deployJobs, deployment, project } = this.props
const { activeSummaryIndex } = this.state
const deployJob = this._getDeployJob()
const server = this._getServer()
const ec2Info = server && server.ec2Info
console.log(server, ec2Info, deployment)
const hasPreviouslyDeployed = deployment.deployJobSummaries.length > 0
return (
<Panel header={<h3><Icon type='server' /> Deployment Summary</h3>}>
Expand Down Expand Up @@ -137,13 +152,14 @@ export default class CurrentDeploymentPanel extends Component<Props, State> {
}
{hasPreviouslyDeployed
? <DeployJobSummary
summary={deployment.deployJobSummaries[activeSummaryIndex]}
deployment={deployment}
downloadBuildArtifact={this.props.downloadBuildArtifact}
project={project}
server={server}
deployment={deployment} />
summary={deployment.deployJobSummaries[activeSummaryIndex]} />
: 'No current deployment found'
}
{ec2Info || deployment.ec2Instances.length > 0
{ec2Info // If has EC2 info, show EC2 instances box.
? <div>
<h4>
EC2 instances ({getActiveInstanceCount(deployment.ec2Instances)} active)
Expand All @@ -154,7 +170,7 @@ export default class CurrentDeploymentPanel extends Component<Props, State> {
<Icon type='refresh' />
</Button>
</h4>
{deployment.ec2Instances.length > 0
{deployment.ec2Instances
? <div style={{ height: '200px', overflow: 'scroll', paddingRight: '10px' }}>
{deployment.ec2Instances.map(instance => {
return (
Expand All @@ -180,6 +196,7 @@ export default class CurrentDeploymentPanel extends Component<Props, State> {
class DeployJobSummary extends Component<{
deployment: Deployment,
downloadBuildArtifact: typeof deploymentActions.downloadBuildArtifact,
project: Project,
server: ?OtpServer,
summary: ?DeploySummary
}> {
Expand Down Expand Up @@ -213,7 +230,7 @@ class DeployJobSummary extends Component<{
}

render () {
const {deployment, server, summary} = this.props
const {deployment, project, server, summary} = this.props
const serverLabel = this._getLabelForServer(server, deployment)
let deploymentType = 'unknown'
if (server) {
Expand Down Expand Up @@ -265,7 +282,10 @@ class DeployJobSummary extends Component<{
</Button>
</ButtonToolbar>
<div style={{ margin: '20px 0' }}>
<DeploymentPreviewButton block deployment={deployment} />
<DeploymentPreviewButton
block
deployment={deployment}
project={project} />
</div>
</div>
)
Expand Down
7 changes: 4 additions & 3 deletions lib/manager/components/DeploymentConfirmModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ import {getFeedNames, versionHasExpired} from '../util/version'
import polygon from 'turf-polygon'
import area from '@turf/area'

import type {Deployment, SummarizedFeedVersion} from '../../types'
import type {Deployment, Project, SummarizedFeedVersion} from '../../types'

type Props = {
deployToTarget: typeof deploymentActions.deployToTarget,
deployment: Deployment,
oldDeployment: ?Deployment,
onClose: () => void,
project: Project,
target: string
}

Expand All @@ -42,7 +43,7 @@ export default class DeploymentConfirmModal extends Component<Props, State> {

render () {
const {Body, Footer, Header, Title} = Modal
const {deployment, oldDeployment, target} = this.props
const {deployment, oldDeployment, project, target} = this.props
const {isDeployed} = this.state
const isMissingFeeds = deployment.feedVersions.length === 0
const isMissingBounds = !deployment.projectBounds
Expand All @@ -57,7 +58,7 @@ export default class DeploymentConfirmModal extends Component<Props, State> {
}
const deploymentIsDisabled = isDeployed || boundsTooLarge || isMissingBounds || isMissingFeeds
const expiredFeeds: Array<SummarizedFeedVersion> = deployment.feedVersions.filter(versionHasExpired)
const server = getServerForId(target, deployment.project)
const server = getServerForId(target, project)
return (
<Modal show onHide={this._onClose}>
<Header>
Expand Down
11 changes: 6 additions & 5 deletions lib/manager/components/DeploymentPreviewButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,22 @@ import Icon from '@conveyal/woonerf/components/icon'
import React, {Component} from 'react'
import {Button, Tooltip, OverlayTrigger} from 'react-bootstrap'

import type {Deployment} from '../../types'
import type {Deployment, Project} from '../../types'

type Props = {
deployment: Deployment
deployment: Deployment,
project: Project
}

export default class DeploymentPreviewButton extends Component<Props> {
render () {
const { deployment, ...buttonProps } = this.props
const { id, deployedTo, project, projectBounds } = deployment
const { deployment, project, ...buttonProps } = this.props
const { id, deployedTo, projectBounds } = deployment
if (!deployedTo) {
// Deployment has not been deployed to a server, do not render button.
return null
}
if (!deployment.project.otpServers) {
if (!project.otpServers) {
console.warn(`No otp servers defined for project`)
return null
}
Expand Down
12 changes: 10 additions & 2 deletions lib/manager/components/DeploymentVersionsTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type {ManagerUserState} from '../../types/reducers'
type Props = {
deleteFeedVersion: typeof deploymentActions.deleteFeedVersion,
deployment: Deployment,
project: Project,
updateVersionForFeedSource: typeof deploymentActions.updateVersionForFeedSource,
user: ManagerUserState,
versions: Array<SummarizedFeedVersion>
Expand All @@ -24,7 +25,14 @@ export default class DeploymentVersionsTable extends Component<Props> {
messages = getComponentMessages('DeploymentVersionsTable')

render () {
const {deployment, deleteFeedVersion, versions, updateVersionForFeedSource, user} = this.props
const {
deployment,
deleteFeedVersion,
project,
versions,
updateVersionForFeedSource,
user
} = this.props
return (
<Table striped hover fill>
<thead>
Expand All @@ -47,7 +55,7 @@ export default class DeploymentVersionsTable extends Component<Props> {
return <FeedVersionTableRow
feedSource={version.feedSource}
version={version}
project={deployment.project}
project={project}
deployment={deployment}
key={version.id}
user={user}
Expand Down
17 changes: 10 additions & 7 deletions lib/manager/components/DeploymentViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ export default class DeploymentViewer extends Component<Props, State> {
{east: 10, north: 10, west: -10, south: -10}
const boundsTooLarge = east - west > BOUNDS_LIMIT || north - south > BOUNDS_LIMIT
const bounds = this._getBounds()
const serverDeployedTo = getServerDeployedTo(deployment)
const serverDeployedTo = getServerDeployedTo(deployment, project)
const isWatchingDeployment = user.subscriptions && user.subscriptions.hasFeedSubscription(
project.id,
deployment.id,
Expand All @@ -279,6 +279,7 @@ export default class DeploymentViewer extends Component<Props, State> {
deployment={deployment}
oldDeployment={oldDeployment}
onClose={this._onCloseModal}
project={project}
target={target} />
}
<Row>
Expand Down Expand Up @@ -428,10 +429,11 @@ export default class DeploymentViewer extends Component<Props, State> {
</p>
: <DeploymentVersionsTable
deleteFeedVersion={deleteFeedVersion}
deployment={deployment}
project={project}
updateVersionForFeedSource={updateVersionForFeedSource}
user={user}
versions={versions}
deployment={deployment} />
versions={versions} />
}
</ListGroup>
</Panel>
Expand All @@ -440,11 +442,12 @@ export default class DeploymentViewer extends Component<Props, State> {
{/* Current deployment panel */}
<CurrentDeploymentPanel
deployJobs={deployJobs}
server={serverDeployedTo}
deployment={deployment}
downloadBuildArtifact={this.props.downloadBuildArtifact}
fetchDeployment={this.props.fetchDeployment}
terminateEC2InstanceForDeployment={this.props.terminateEC2InstanceForDeployment}
deployment={deployment} />
project={project}
server={serverDeployedTo}
terminateEC2InstanceForDeployment={this.props.terminateEC2InstanceForDeployment} />
{/* Configurations panel */}
<Panel header={<h3><Icon type='cog' /> OTP Configuration</h3>}>
<ListGroup fill>
Expand Down Expand Up @@ -601,7 +604,7 @@ class CustomConfig extends Component<{
disabled={!this.state[name] || !validJSON}
onClick={this._onSaveConfig}>Save</Button>
: <LinkContainer
to={`/project/${deployment.project.id}/settings/deployment`}>
to={`/project/${deployment.projectId}/settings/deployment`}>
<Button bsSize='xsmall'>
<Icon type='pencil' /> Edit
</Button>
Expand Down
6 changes: 3 additions & 3 deletions lib/manager/components/DeploymentsPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,8 @@ class DeploymentListItem extends Component<RowProps> {
}

render () {
const {deployment} = this.props
const server = getServerDeployedTo(deployment)
const {deployment, project} = this.props
const server = getServerDeployedTo(deployment, project)
const na = (<span className='deployment-na'>N/A</span>)
return (
<tr className={this._isPinned() ? 'pinned-deployment' : ''}>
Expand All @@ -263,7 +263,7 @@ class DeploymentListItem extends Component<RowProps> {
value={deployment.name}
rejectEmptyValue
onChange={this._onChangeName}
link={`/project/${deployment.project.id}/deployments/${deployment.id}`}
link={`/project/${deployment.projectId}/deployments/${deployment.id}`}
/>
</td>
<td>
Expand Down
6 changes: 3 additions & 3 deletions lib/manager/components/ProjectViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ export default class ProjectViewer extends Component<Props, State> {
} else {
browserHistory.push(`/project/${id}/${key}`)
}
if (key === 'deployments' && !deployments) {
this._fetchDeployments()
}
// if (key === 'deployments' && !deployments) {
// this._fetchDeployments()
// }
}

shouldComponentUpdate (newProps: Props, nextState: State) {
Expand Down
3 changes: 2 additions & 1 deletion lib/manager/components/version/FeedVersionNavigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,8 @@ export default class FeedVersionNavigator extends Component<Props, State> {
<ButtonGroup>
{isModuleEnabled('deployment') && deploymentForVersion
? <DeploymentPreviewButton
deployment={deploymentForVersion}>
deployment={deploymentForVersion}
project={project}>
View deployment {version && version.id}
</DeploymentPreviewButton>
: null
Expand Down
2 changes: 1 addition & 1 deletion lib/manager/containers/ActiveFeedVersionNavigator.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export type Props = {
editDisabled?: boolean,
feedSource: Feed,
isPublic?: boolean,
project?: Project,
project: Project,
routeParams: RouteParams,
versionIndex?: number
}
Expand Down
2 changes: 1 addition & 1 deletion lib/manager/reducers/projects.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ const projects = (state: ProjectsState = defaultState, action: Action): Projects
}
case 'RECEIVE_DEPLOYMENT': {
const {projectIndex} = getIndexesFromFeed({
projectId: action.payload.project.id,
projectId: action.payload.projectId,
state
})
const existingDeployments = state.all[projectIndex].deployments || []
Expand Down
4 changes: 2 additions & 2 deletions lib/manager/util/deployment.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,9 @@ export const EC2_INFO_FIELDS: Array<FormProps> = [
}
]

export function getServerDeployedTo (deployment: Deployment) {
export function getServerDeployedTo (deployment: Deployment, project: Project) {
return deployment.deployedTo
? getServerForId(deployment.deployedTo, deployment.project)
? getServerForId(deployment.deployedTo, project)
: null
}

Expand Down
Loading

0 comments on commit 9b66f89

Please sign in to comment.