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

Save state #205

Merged
merged 14 commits into from Sep 24, 2020
Merged

Save state #205

merged 14 commits into from Sep 24, 2020

Conversation

BeckySharp
Copy link
Contributor

As with previous PRs and the discussion, there the engine adds the mentions to its state, the State lasts the life of the Engine, and optionally can be persisted or dumped.

  • Not all methods are in place (e.g., some SQLState methods and the jsonlines serialization of Mentions)
  • I'd like someone to check/help write tests for the Ordering adaptation for Mentions vs ResultsItems in MemoryState (for the TreeSet)
  • I removed the StateFactory (didn't seem necessary)
  • I put the ResultsIterator in the MentionFactory, since the Iterator creation is where the Mentions get built, so the MentionFactory seemed like a good place to do it
  • I passed the IndexSearcher to SQLState. This is because the LazyIdGetter needs access to the IndexSearcher, and that's needed to make Mentions, and the SQLState essentially re-makes mentions when asked to "getAll/Some"

Please review and we can discuss!! :)

@BeckySharp
Copy link
Contributor Author

for the sake of a single state branch, merging

@BeckySharp BeckySharp merged commit efb53be into kwalcock-noWriteStateInQuery Sep 24, 2020
@BeckySharp BeckySharp deleted the save_state branch September 24, 2020 23:13
@kwalcock
Copy link
Contributor

These squashing merges aren't working for me at all. Based on the commit messages alone, this is where the code has ended up and where new code should be added. This branch also very much needs a different name.

@kwalcock
Copy link
Contributor

I can rename if this is the place to do it. How about stateV2?

@BeckySharp
Copy link
Contributor Author

BeckySharp commented Sep 25, 2020 via email

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

Successfully merging this pull request may close these issues.

None yet

2 participants