It's a very minor improvement in terms of performance or memory but figured this is executed every time we fetch a report so may be better to directly fetch the right result and be easier to debug. While debugging some archiving memory issues noticed this query returns often 3000 results (depends on the number of segments etc... can be more or less). It's mostly saving PHP time and less time in "sending data"
I can kind of get the tests to work but I'm quite certain there is a bug in the current archive selection. Might need to do some call some day to understand things why the current implementation is.
I'm not 100% sure but I think I can see that we falsely assume a site might have an invalidated archive when there is not (since we partially except any done flag)... leading potentially to unneeded archive invalidations maybe...
And we partially use possibly maybe the wrong visits/visits converted values in circumstances although this one might be fine.
Maybe I come back to this at some point but might just close it as it's hard to really understand how it is supposed to work and not to work
@tsteur fixed the tests, not sure why the UI tests are failing, but it seems unrelated
fyi @diosmosis changed the base to 4.x-dev and fixed the merge conflicts so will see what the tests do... there were 2 conflicts in tests so I expect to see some failures there where likely only need to adjust the fixtures.
Thanks for triggering the build @diosmosis didn't realise it wasn't automatically running another built when changing the base to 4.x-dev.
It's doing the funny thing again where it can't run the builds... same that happened in the other build that was updating the consent page. And just like in the other PR all tests pass in the PUSH build even though we'd probably expect them to fail. In https://github.com/matomo-org/matomo/pull/15973 the PUSH build was also green but it couldn't have been green because the page was changed. Once merged, the failure was visible in the 4.x-dev branch.
So I reckon we might need to merge this PR, and then we can probably some new test failures in 4.x-dev branch from the merge commit in https://github.com/matomo-org/matomo/pull/15887/commits/6c2fa2ab372b5f8c4530500b289281dc631b4207#diff-dcb3557abe94edd8776f637afdfb76b7R103
Go for it
On Mon, 29 Jun 2020, 16:58 diosmosis, <firstname.lastname@example.org> wrote:
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
4.x-dev build looks ok merged, just some minor things unrelated to this pr