Conversation
|
Wasn't sure who to request review from, hopefully one of you is correct. 😄 |
neilisfragile
left a comment
There was a problem hiding this comment.
These changes look good to merge as is.
While you are in the area could you also check if other datapoints are transmitted and not currently handled?
For instance I think count_monthly_users is an example.
For double points, if you could also add aggregation support for count_monthly_users that would be extremely helpful. https://github.com/matrix-org/panopticon/blob/master/scripts/aggregate.py it should be quite easy to crib.
No need to aggregate log level though.
I cross-referenced and it seems I'll add that and take a look at aggregating it. |
|
@neilisfragile I think I made the proper changes, the |
neilisfragile
left a comment
There was a problem hiding this comment.
LGTM, I'm afraid that the testing coverage is pretty poor though changes are generally rare - the aggregate table will also need the mau column adding manually.
|
This was deployed in release v0.1.3 and I confirmed that both new columns are having data added. |
This matches the changes from matrix-org/synapse#8477.