-
Notifications
You must be signed in to change notification settings - Fork 44
Bug 1520857- add support in tc-web for displaying information from the lastFire table #413
base: master
Are you sure you want to change the base?
Changes from 1 commit
c57174f
7a33df8
5b5c97d
a154a50
f55678d
c5f5a16
87993e3
f98d385
4674382
0156365
e238f6f
8bb249f
2b28062
aa51ef9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,10 @@ import { equals, assocPath } from 'ramda'; | |
import cloneDeep from 'lodash.clonedeep'; | ||
import CodeEditor from '@mozilla-frontend-infra/components/CodeEditor'; | ||
import Code from '@mozilla-frontend-infra/components/Code'; | ||
import ExpansionPanel from '@material-ui/core/ExpansionPanel'; | ||
import ExpansionPanelSummary from '@material-ui/core/ExpansionPanelSummary'; | ||
import ExpansionPanelDetails from '@material-ui/core/ExpansionPanelDetails'; | ||
import ExpandMoreIcon from '@material-ui/icons/ExpandMore'; | ||
import { withStyles } from '@material-ui/core/styles'; | ||
import List from '@material-ui/core/List'; | ||
import ListItem from '@material-ui/core/ListItem'; | ||
|
@@ -184,6 +188,8 @@ export default class HookForm extends Component { | |
validation: {}, | ||
}; | ||
|
||
expandsionSummary = React.createRef(); | ||
|
||
static getDerivedStateFromProps(props, state) { | ||
if ( | ||
equals(props.hook, state.previousHook) && | ||
|
@@ -439,8 +445,8 @@ export default class HookForm extends Component { | |
hookLastFires, | ||
validation, | ||
} = this.state; | ||
/* eslint-disable-next-line no-underscore-dangle */ | ||
|
||
/* eslint-disable-next-line no-underscore-dangle */ | ||
return ( | ||
<Fragment> | ||
<List> | ||
|
@@ -544,6 +550,7 @@ export default class HookForm extends Component { | |
<DataTable | ||
items={hookLastFires} | ||
headers={['Task ID', 'FiredBy', 'Result', 'Attempted', 'Error']} | ||
isPaginate | ||
renderRow={hookFire => ( | ||
<TableRow key={hookFire.taskId}> | ||
<TableCell> | ||
|
@@ -566,11 +573,21 @@ export default class HookForm extends Component { | |
</TableCell> | ||
<TableCell className={classes.errorTableCell}> | ||
{(hookFire.result === 'ERROR' && ( | ||
<ErrorPanel | ||
className={classes.errorPanel} | ||
error={hookFire.error} | ||
onClose={null} | ||
/> | ||
<ExpansionPanel> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a hook I can see where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of having an expansion panel for each error, we should instead have a button that opens a drawer with the error on click. This is similar to https://taskcluster-web.netlify.com/provisioners/aws-provisioner-v1 when clicking the info icon. The drawer can display the |
||
<ExpansionPanelSummary | ||
expandIcon={<ExpandMoreIcon />}> | ||
<Typography className={classes.heading}> | ||
Show/Hide | ||
</Typography> | ||
</ExpansionPanelSummary> | ||
<ExpansionPanelDetails> | ||
<ErrorPanel | ||
className={classes.errorPanel} | ||
error={hookFire.error} | ||
onClose={null} | ||
/> | ||
</ExpansionPanelDetails> | ||
</ExpansionPanel> | ||
)) || <Typography>N/A</Typography>} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use lower case |
||
</TableCell> | ||
</TableRow> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we adding pagination? Is the last fire endpoint paginated (i.e., uses continuation token)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per discussion with Dustin in irc we are keeping as many as 100 lastfire info rows for each hookId (expiration logic). Hence I think showing all rows at once won't be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also only purge those once per day, so a particularly active hook might have many more rows than that at some time during the day!