-
Notifications
You must be signed in to change notification settings - Fork 207
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
Fix issues with PerformanceReport Page (commit data, hiding missing data) #1461
base: performance-benchmark-page
Are you sure you want to change the base?
Fix issues with PerformanceReport Page (commit data, hiding missing data) #1461
Conversation
d48f203
to
0712349
Compare
…ata) and increase Axios timeout
0712349
to
465a163
Compare
@@ -5,7 +5,7 @@ import axios from 'axios'; | |||
|
|||
export const AxiosConfig = axios.create({ | |||
baseURL: process.env.REACT_APP_LAMBDA_URL, | |||
timeout: 3000, | |||
timeout: 5000, |
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.
what is this based on can we document it? can we also document that this is in milli seconds
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.
The trends page often runs into a timeout when collecting commit data.
Will update this to add retry/show an error page as well.
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.
minor comments
// commit_title: `${commit_info?.title} (#${commit_info?.number})`, | ||
// commit_url: commit_info?.html_url, | ||
commit_title: `${commit_info?.title} (#${commit_info?.number})`, | ||
commit_url: commit_info?.html_url, |
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.
can we add default fallbacks here to make sure it doesn't break
|
||
setState((prev: any) => ({ | ||
...prev, | ||
version: service_info.tag_name, | ||
ami_id: ami_id, | ||
collection_period: collection_period, | ||
use_cases: use_cases, | ||
// commit_title: `${commit_info?.title} (#${commit_info?.number})`, | ||
// commit_url: commit_info?.html_url, | ||
commit_title: `${commit_info?.title} (#${commit_info?.number})`, |
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.
can we add default fallbacks here to make sure it doesn't break
.at(0); | ||
const commit_history = selected_hash_information?.commit.message.replace(/\n\r*\n*/g, '<br />'); | ||
const commited_by = `Committed by ${selected_hash_information?.author.login} on ${selected_hash_information?.author.date}`; | ||
const commited_by = `Committed by ${selected_hash_information?.author.login} on ${selected_hash_information?.commit?.author?.date}`; |
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.
can we add default fallbacks here to make sure it doesn't break
<StyledTableCell>{Number(use_case.data?.[data_rate]?.net_bytes_sent).toFixed(2)}</StyledTableCell> | ||
<StyledTableCell>{Number(use_case.data?.[data_rate]?.net_packets_sent).toFixed(2)}</StyledTableCell> | ||
</StyledTableRow> | ||
) : null; |
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.
is it better to return null than <>
component ?
This PR was marked stale due to lack of activity. |
Description of the issue
Fixes issues with PerformanceReport Page
Description of changes
How does this change address the problem?
License
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Tests
Describe what tests you have done.
Requirements
Before commit the code, please do the following steps.
make fmt
andmake fmt-sh
make lint