Skip to content
This repository has been archived by the owner on Sep 28, 2022. It is now read-only.

WIP: Import options - better gopilosa.StatusChan support #83

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

travisturner
Copy link
Member

I'm just putting this PR here so you can see what I changed in the PDK in order to get the StatusChan to work. It builds on top of #78. There are still some issues. It assumes that a frame contains only one of:

  • a standard bit frame
  • a single BSI field
    Even with this limitation, and protection in the PDK, there is a still a potential panic situation (closing of a closed channel) in go-pilosa that I marked with a TODO in a PR to that repo.

The general idea here is that StatusChan is frame-specific, so I made it part of the FrameSpec instead of provided as a functional option for the entire index in SetupPilosa().

I don't think it's necessary to merge this PR into master, but we'll need to consider all of this as we refactor Pilosa. I think the Frame to Field logic will help because that will effectively enforce the limitations listed above.

@travisturner travisturner self-assigned this May 24, 2018
@travisturner travisturner requested review from yuce and jaffee May 24, 2018 16:13
@travisturner
Copy link
Member Author

fixes #79

@jaffee
Copy link
Member

jaffee commented May 24, 2018

a decision on FeatureBaseDB/go-pilosa#141 will have a significant bearing on this.

passing the statusChan in the importOptions per frame seems fine to me. Note that the PDK will also auto-create frames if they aren't set up initially. We might consider adding a method to the Indexer to get status chans for these frames at some point.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants