Moved analytics configuration to the database. #551
Conversation
| @@ -978,11 +979,13 @@ def apply(self, _db, collection, replace=None): | |||
| if pool and self._availability_needs_update(pool): | |||
| # Update availabily information. This may result in | |||
| # the issuance of additional circulation events. | |||
| analytics = Analytics(_db) | |||
There was a problem hiding this comment.
I don't like this, but I'm not sure anything else would be better.
There was a problem hiding this comment.
Yeah, this is a performance problem. We're looking at database queries and module imports every time we want to change a LicensePool. I think analytics should be passed into ReplacementPolicy just like mirror is. That should cut down on the number of times we have to instantiate a new Analytics object.
|
Also added in two sitewide settings that weren't in the admin interface yet. |
analytics.py
Outdated
| @@ -30,5 +37,5 @@ def __init__(self, _db): | |||
| def collect_event(self, library, license_pool, event_type, time=None, **kwargs): | |||
| if not time: | |||
| time = datetime.datetime.utcnow() | |||
| for provider in self.providers: | |||
| for provider in (self.sitewide_providers + self.library_providers[library.id]): | |||
There was a problem hiding this comment.
When you call collect_event() several times to trigger the same event for multiple libraries, will this cause the event to be collected once for each sitewide provider?
If so, it might work better for the sitewide providers to be handled as providers that, if enabled, are presumed to be enabled for every library in the system. The event still happens multiple times, but it's recorded as happening in each relevant library.
Or maybe it doesn't matter much because the circulationevents table doesn't record the library ID, only the license pool ID, so collecting an several times will do the same thing as collecting it once.
There was a problem hiding this comment.
That's a good point. If there's a collection with multiple libraries it will cause duplicate events in the sitewide provider. I was thinking that I'd have to add the library id to the CirculationEvents table and the Google Analytics setup in a future pr though.
There was a problem hiding this comment.
Adding library ID to CirculationEvents makes a lot of sense. File a bug for that and then 👍
This branch moves the analytics configuration into an ExternalIntegration, and changes Analytics so its no longer a global singleton. It also defines the constants for LocalAnalyticsProvider to be configured in the admin interface.