-
-
Notifications
You must be signed in to change notification settings - Fork 295
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
Performance telemetry API #2490
base: master
Are you sure you want to change the base?
Conversation
0b63d52
to
955549a
Compare
$('#id_before').on('dp.change', reloadCharts) | ||
document.getElementById('id_test_plan').onchange = reloadCharts | ||
document.getElementById('id_product').onchange = () => { | ||
updateTestPlanSelectFromProduct(reloadCharts) |
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.
When changing the product I get error
Internal error: Cannot resolve keyword 'category' into field. Choices are: assignee, assignee_id, bugs, build, build_id, case, case_id, case_text_version, id, linkreference, run, run_id, sortkey, start_date, status, status_id, stop_date, tested_by, tested_by_id
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.
fixed.
'black', | ||
'lightBlue', | ||
'lightGreen' | ||
] |
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.
This should be managed automatically by the charting library (d3 IIRC), no ?
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.
yes. removed
const groupedCategories = [] | ||
// map of user ID -> table column. we use map here for faster lookup by user ID. | ||
const groupedColumnsDataMap = {} | ||
const userIds = new Set() |
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.
You need user names. "User 5" doesn't mean anything.
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.
added.
assignee_count[assignee] = 1 | ||
|
||
if execution["run_id"] != run_id: | ||
result[run_id] = assignee_count |
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.
This can either be "TR-123" directly here or adjust the JS code to render the coordinates on the X axis as TR-123.
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.
done. I changed it on the UI level.
console.log(result) | ||
|
||
// the actual result is in the same format, only it can be much bigger | ||
// and the chart may break |
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 need to have an understanding of why & how this chart breaks. If you are seeing breakage locally in devel, that's not a terribly large dataset.
Both Status matrix & Execution trends pages seem to have a larger dataset, at least in production and they seem to load just fine. On tcms.kiwitcms.org Execution trends takes a few seconds but it contains info from 5564 executions across around 1000 test runs grouped by statuses.
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.
I meant this comment for here - #2490 (comment)
tcms/telemetry/api.py
Outdated
# count only execution which are finished | ||
.exclude(status__weight=0) | ||
.order_by("run_id") | ||
.values("assignee_id", "run_id") |
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.
Wouldn't an aggregate query with Count and group by be a better choice here?
And if there isn't a big difference you can also add assignee__username into the mix.
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 charts looks like this in my setup
this is for 349 executions. I am not able to find how to tell C3 to render some small part of the chart normally and add a horizontal scroll that leads to the other part. moreover, I found this issue that asks for something similar, which is still open - c3js/c3#1702 there is a workaround in the issue, I will try to do something will that
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.
Wouldn't an aggregate query with Count and group by be a better choice here?
can you clarify more? what should we count by?
still WIP, because the page breaks when given too much data. the algorithm for the data looks OK, because it works with the small data-set I left in the code. however, given dataset with more than 500 runs it fails to display the chart miserably.
- API error, because of wrong key - remove colors, because that is handled by C3 - add usernames, not user IDs - add 'TR-' notation
955549a
to
88d212f
Compare
This API will going to be used for the Performance telemetry page.
It outputs result with format:
the top-level keys are run IDs. each run ID has associated map of assignee ID - count.
this result means that for run with ID
2
there is 1 test execution, assigned to use with ID1
.null
for assignee ID means that no one is assigned for this run. we may or may not want to filter out this data.