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

Completely decouple Harvesting Utils from Broker Microservice #9

Open
lukehesluke opened this issue Mar 5, 2024 · 1 comment
Open

Comments

@lukehesluke
Copy link
Contributor

lukehesluke commented Mar 5, 2024

Harvesting Utils was originally moved from Broker Microservice.

Much of the Broker Microservice-specific logic was abstracted in order to do this, but the code is still coupled in some places. Ideally, this should all be removed.

Impact

I believe that Harvesting Utils cannot move to v1 until this is resolved

Issues

Some known issues (some were spawned from this comment):

Remove the need for state.feedContextMap

  1. Move the Duplicate feed identifier not permitted within dataset distribution check to Broker
  2. I'm not fully sure I understand the reason for the feedContextMap.delete(..) logic when a 404 is reached. But we could instead employ a callback like onFeed404Error(..), where Broker would put its feedContextMap.delete(..)

Remove the need for state.startTime

startTime looks like it's just used when the last page is reached. So this could just be replaced with an onLastPage(..) callback, and Broker could do its own wrangling.

Remove the need for state.context

This is a bit more complicated of course!

Remove the need for multibar

This assumes that Harvesting Utils users will be:

  • Creating a progress bar
  • Creating that progress bar with the multibar library

Instead, there should be some way of reporting progress that Broker can tap into, such that only Broker manages multibar and other means for reporting that progress (e.g. with a different library or simpler logging) can tap into this.

Remove the reference to FatalError

See the code comment next to the check for "FatalError"

@nickevansuk
Copy link
Contributor

Consider addressing this todo as part of this also:

// TODO: To prevent extraneous output, ensure that multibar.stop is only called if the multibar is already active (e.g. by wrapping the multibar to allow us to set it to null when not in use)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 💡Ideas
Development

No branches or pull requests

2 participants