Skip to content
This repository has been archived by the owner on Dec 1, 2024. It is now read-only.

Break levelup into smaller packages #270

Closed
6 of 7 tasks
rvagg opened this issue Aug 6, 2014 · 7 comments
Closed
6 of 7 tasks

Break levelup into smaller packages #270

rvagg opened this issue Aug 6, 2014 · 7 comments
Labels
discussion Discussion

Comments

@rvagg
Copy link
Member

rvagg commented Aug 6, 2014

This is intended as a follow-up to the discussion in #255 where we have been unable to reach enough of a consensus to merge the proposed feature. The approach outlined in this issue has been suggested as something of a compromise and an attempt to provide the Dulcimer community with something useful without going against the fundamental ideas of LevelUP that some have stated as their main objection to the merge.

One major problem we are bumping up against is the inability to get at the deep insides of LevelUP itself. While we have obviously achieved a good API that is extensible, otherwise there wouldn't be such a vibrant ecosystem, we have extensibility problems at a deeper level, with too much logic tucked away internally with no means of accessing it.

Plus, LevelUP turns out to be too big for most of our tastes, even the fact that we have a lib directory is a warning sign. We're already removing WriteStream and opting for a user-lang pluggable mechanism, we should be able to take a similar approach with all of the components of LevelUP. If we modularise properly we can leave LevelUP as a simple glue between the main parts that form what we know of as LevelUP today but with the logic of each of those parts residing in self-contained, separately tested and documented packages. We can perform most of this work in the Level org too.

Some things this would enable:

  • Wiring together alternative forms of LevelUP by piecing together the components in different ways with different glue logic, perhaps pulling in additional components, perhaps removing some and perhaps replacing some with alternative versions.
  • Experiment more flexibly with alternative approaches
  • Iterate faster by forming interest groups around the separate packages and adopting separate release cycles
  • Re-use some of the great logic that's currently tucked away inside LevelUP, for example level-sublevel pulls in all of LevelUP now to just reuse a couple of the internal components.
  • Do something meaningful with the leveldb package that we now own and have yet to do anything with! (a more strict interpretation of leveldb exposed in Node? A more fully-featured LevelUP?)

Initial suggestion for package split-up:

  • codec/encodings - for k/v encoding and decoding
  • read-stream
  • write-stream (already done, in multiple forms)
  • errors (maybe)
  • batch (not sure how this would work, perhaps push more of it down into abstract-leveldown?)

Also a bunch of smaller things such as:

  • the logic for keeping track of the open/close state add .status abstract-leveldown#68
  • the /NotFound/ matching stuff, which I have already re-implemented in other places myself

And doing a re-audit of the logic that is currently duplicated across levelup and abstract-leveldown, particularly the argument checking. In fact, an abstract-levelup might even be in order purely to define the levelup interface and provide a basic shell for it.

Discussion and humorous animated GIFs please.

@adamavenir
Copy link

@rvagg, you are an absolute class act as a project maintainer / lead. 👑🎩🌟

@juliangruber
Copy link
Member

👍 for factoring out read-stream

Maybe another way to think about this is to take a use case and see what is the least possible amount of work to be done to make it work...instead of modularizing without knowing the absolute gain and use cases in need of that refactor.

Taking the (extra) arg requirement is an interesting one as it needs hooks deep into level. I imagine something like this:

db.post.read(function(args){
  var key = args[0];
  var value = args[1][0];
  var extra = args[1][1];
  return [key, value, extra];
});

Basically hooks before and after each operation.

@dominictarr
Copy link
Contributor

Yes this would be great!
read-stream and encodings could pretty much just be copied to new projects and published as they are.
after a recent pull request there is now not very much coupling between those - and actually, level-sublevel already uses them directly (require('levelup/lib/encodings') and require('levelup/lib/read-stream')) this makes it easy to ensure a compatibility with levelup, while also only maintaining read-streams etc in just one place. The old level-sublevel rewrote too much of levelup and that was a problem.

In the new version level-sublevel still basically rewrites the LevelUP object, which is mostly about
handling optional argument patterns, and calling the encodings. I wonder if this could just have a some properties: _preput, _predel, _prebatch that could be called to handle that? with probably something similar for _post*. Having a method that you just override would be the least overhead, this would enable a registerPrePut method like in dulcimer, but if you didn't need that it's no overhead.

hmm, so maybe the _premethod returns gets to call leveldown, and then the callback isn't the user cb, but is one that calls the _postput , and then calls the user cb with that result?

@max-mapper
Copy link

referencing #273 from here as I think it's part of this initiative

edit thanks neonstalwart for fixing typo in this comment

@juliangruber
Copy link
Member

for overview i turned the bullet points in rod's post into checkboxes

@ralphtheninja
Copy link
Member

I ticked the "the logic for keeping track of the open/close state" /cc @juliangruber

@ralphtheninja
Copy link
Member

I think we have arrived at a point where we can say that this has been achieved, levelup is no longer depending on encodings etc.

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

No branches or pull requests

6 participants