You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A huge amount of work was done to decouple Harvesting Utils and Broker Microservice in #9.
However this issue still remains.
Do not share FeedContexts
Harvesting Utils has a FeedContext object, which it uses to track harvesting progress (perhaps it should even be renamed HarvestProgress?)
Broker also wants to tracks harvesting progress — so that it can render this information in its /status route — as well as other bits of data (like a multibar progress bar, etc) for each feed. It does this by:
Having its own FeedContext type, which is the same as Harvesting-Utils', and also adds extra fields
Before initiating the harvest of a feed, Broker initiates its own context and then passes that context to Harvesting-Utils using the overrideContext arg.
The result is that this one object is updated by both Harvesting-Utils and Broker. This is overly coupled design and means that changes to Harvesting-Utils could break Broker and vice versa.
Suggested Fix
Harvesting-Utils: Removes the overrideContext arg (which is already marked as deprecated); AND no longer exposes the createFeedContext(..) function.
Harvesting-Utils: Include a copy of its internal context into the various callbacks (e.g. processPage(..))
Broker: Maintains its own FeedContext type, which is no longer derived from Harvesting Utils
Broker: In the callbacks that it sends to harvestRPDELossless(..), it should use the context object that Harvesting-Utils shares (see step 2) to update its own context with the progress info that it needs for its progressbar and its /status endpoint.
ALSO:
Move totalItemsQueuedForValidation, validatedItems, sleepMode, timeToHarvestCompletion out of Harvesting-Utils' FeedContext definition, and into Broker's. These fields are only relevant to Broker.
The text was updated successfully, but these errors were encountered:
A huge amount of work was done to decouple Harvesting Utils and Broker Microservice in #9.
However this issue still remains.
Do not share FeedContexts
Harvesting Utils has a
FeedContext
object, which it uses to track harvesting progress (perhaps it should even be renamedHarvestProgress
?)Broker also wants to tracks harvesting progress — so that it can render this information in its
/status
route — as well as other bits of data (like amultibar
progress bar, etc) for each feed. It does this by:FeedContext
type, which is the same as Harvesting-Utils', and also adds extra fieldsoverrideContext
arg.The result is that this one object is updated by both Harvesting-Utils and Broker. This is overly coupled design and means that changes to Harvesting-Utils could break Broker and vice versa.
Suggested Fix
overrideContext
arg (which is already marked as deprecated); AND no longer exposes thecreateFeedContext(..)
function.processPage(..)
)harvestRPDELossless(..)
, it should use the context object that Harvesting-Utils shares (see step 2) to update its own context with the progress info that it needs for its progressbar and its/status
endpoint.ALSO:
totalItemsQueuedForValidation
,validatedItems
,sleepMode
,timeToHarvestCompletion
out of Harvesting-Utils' FeedContext definition, and into Broker's. These fields are only relevant to Broker.The text was updated successfully, but these errors were encountered: