Skip to content
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

Add ability for Archivers to initiate archiving for other plugins & use in Goals #13105

Merged
merged 13 commits into from Jul 19, 2018

Conversation

diosmosis
Copy link
Member

@diosmosis diosmosis commented Jun 25, 2018

Changes:

  • Add Archiver:: processDependentArchive() method that can be used by plugin archivers that depend on reports in other plugins. Ensures that the archive is processed w/ a segment just for that plugin, and not, eg, for every plugin.
  • Add ArchiveProcessor\Params::$isRootArchiveRequest used to identify if a request is the initial archiving request or if it is part of processDependentArchive().
  • Add Segment::combine() method that combines one segment w/ another.

Fixes #12706

@diosmosis diosmosis added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Jun 25, 2018
@diosmosis diosmosis added this to the 3.6.0 milestone Jun 25, 2018
@diosmosis
Copy link
Member Author

Closing for now, identified performance issue w/ these changes

@diosmosis diosmosis closed this Jun 25, 2018
@diosmosis diosmosis reopened this Jun 28, 2018
@diosmosis diosmosis changed the title Add required segments for Goals.get to known segments to archive list. Add ability for Archivers to initiate archiving for other plugins & use in Goals Jun 28, 2018
@diosmosis
Copy link
Member Author

FYI @tsteur modified this PR to initiate archiving for dependent plugins.

Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks good, but tests need an update

@@ -19,6 +19,7 @@
use Piwik\Plugins\CoreHome\SystemSummary;
use Piwik\Tracker\GoalManager;
use Piwik\Category\Subcategory;
use Piwik\Plugins\VisitFrequency\API as VisitFrequencyAPI;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guess that can be removed again

@diosmosis diosmosis force-pushed the 12706-prearchive-goals-segments branch from faf32e0 to ac98811 Compare July 3, 2018 23:40
@diosmosis diosmosis force-pushed the 12706-prearchive-goals-segments branch from 0845a6d to 8626353 Compare July 4, 2018 00:35
@diosmosis
Copy link
Member Author

Updated.

@sgiehl
Copy link
Member

sgiehl commented Jul 9, 2018

@diosmosis there are still a few failing tests in PrivacyManager plugin

@diosmosis
Copy link
Member Author

Updated the data purging test & verified the new numbers were correct

core/Segment.php Outdated
if (empty($segmentCondition)
|| strpos($segment, $operator . $segmentCondition) !== false
|| strpos($segment, $segmentCondition . $operator) !== false
|| $segment === $segmentCondition
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just wondering... here we may need to also check against urlencoded values and urldecoded? It is this funny thing with segments that they may be urlencoded or not for some reason. No idea whether this would be a thing here during archiving or if they have all same format. Just wanted to mention it.

@diosmosis diosmosis merged commit 2020b12 into 3.x-dev Jul 19, 2018
@diosmosis diosmosis deleted the 12706-prearchive-goals-segments branch July 19, 2018 02:03
InfinityVoid pushed a commit to InfinityVoid/matomo that referenced this pull request Oct 11, 2018
…se in Goals (matomo-org#13105)

* Add required segments for Goals.get to known segments to archive list.

* Add test to ArchiveCronTest.

* Allow archiving dependent archives in Goals Archiver.

* Clean up last commit & use Segment::combine in more places.

* skip dependent processing if same plugin/segment

* Move ArchiveCronTest to CoreConsole plugin since it is rather long running.

* Fixing tests.

* Remove use statements.

* Fix tests dependent on archive tables.

* check w/ urlencoded/decoded segment/condition in Segment::combine().

* Another test fix

* final test fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants