Compatibility with abstract-level v.2.0.0?

[dehmer]

TL;DR: Is level-read-stream v1.1.0 compatible with abstract-level v.2.0.0? If not, is there a similar package which supports Node Streams and is compatible with abstract-level v.2.0.0?

I'm trying to migrate a project to current Level-APIs. The project heavily uses Level (native) for different aspects under Node.js/Electron (with node integration enabled). Besides other stuff, I have a class extending AbstractLevel with an iterator extending AbstractIterator. This class basically uses two sublevels with different value encodings (JSON and Well-known Binar, WKB) to split and merge data between these two sublevels based on same keys.

The implementation is based on the v2.0.0 abstract-level (Promise) API. The AbstractLevel part works like a charm. AbstractIterator when combined with level-read-stream does more or less nothing.

EntryStream seems not to emit any event for the following code:

/**
 * list :: EntryStream -> [{k, v}]
 * @async
 */
const list = stream => new Promise((resolve, reject) => {
  const acc = []
  stream
    .on('data', data => acc.push(data))
    .on('err', reject)
    .on('end', () => resolve(acc))
})

I tried to debug the code for Node.js, but came to no definitive conclusion. It seems that the "handoff" to Readable somehow fails. The iterator itself is producing the correct output for _next().

Good news! Working code ahead:

/**
 * list :: AbstractLevel -> [{k, v}]
 * @async
 */
const list = async db => {
  const acc = []
  for await (const entry of db.iterator()) acc.push(entry)
  return acc
}

So the iterator code might not be broken after all.

If you think that level-read-stream v1.1.0 is indeed compatible with current abstract-level then I happily patch together a minimal repository which demonstrates the issue. In the meantime I'll try to go with for await.

Thanks for your attention and all the good work you put into the Level ecosystem.

[vweevers]

Are you specifically looking to read multiple entries at once (in which case I'd recommend iterator.all()) or was that just an example to demonstrate the potential bug? Which yes, does look like a bug.

level-read-stream should be compatible with abstract-level 2.0.0 though I wonder which concrete implementation you're using because level and its friends have not been updated to 2.0.0 yet.

[dehmer]

Hey! Thank for the REALLY quick reply!

If my memory serves me well, we are pumping GeoJSON through streams to OpenLayers. I sort of like streams because of, well the streaming. Meaning async, buffered, lots of time for event loop to do other stuff, back-pressure handing and so on. But it is certainly not a must, mostly a matter of habit.

I try to put some simple "DelegatingLevel" or something together which might hopefully expose the problem. Usually that's the time I recognize what's wrong with MY code ;-) I'll report back when I have a repository available. In the meantime: Cheers!

[dehmer]

I put together a little something: https://github.com/dehmer/cb46bd75 The issue I observe still remains. Also I notices some "conflicting" versions while npm installing (see README.md). If you'd be so kind to have a look. I want to apologize beforehand for stupid mistakes on my part. BTW: This is not a time sensitive matter. We can stick to the previous Level API for as long as it's necessary.

In the first skipped test case EntryStream is piped to a Writable. In the second skipped test case events are consumed directly.

Thanks for your interest and time for helping me out!

[vweevers]

Ah, level-read-stream uses the old callback API, which is gone in abstract-level v2. So my bad, they're not actually compatible. Needs to be fixed here. Not sure how yet.

[dehmer]

OK. Just what I thought. No worries! Thanks for the confirmation. 👍

From your PR I deduce that you plan to support both, callback and promise, APIs with the upcoming version. I don't know how other abstract-level depend modules like memory-level or others deal with this. My gut tells me this is unnecessary and only increases (accidental) complexity for the new version. One can just use level-read-stream v1.x for callback and v2.x for promise API. On the other hand, I have next to no experience managing and versioning modules with high visibility and a huge user base.

I will have a look at abstract-level-2 branch and report back my findings.

BTW: Node 12, 14 and 16 are end-of-life. Maybe you could update the actions/checks accordingly.

[vweevers]

My gut tells me this is unnecessary and only increases (accidental) complexity for the new version.

Yeah, I considered it. The complexity is worth it because abstract-level v2 itself already contains several significant changes (which will extend to major releases of memory-level, classic-level, level and more) so I don't want add more caveats & work for consumers - when it's easily avoided. Modules are meant to hide complexity. Down the road we'll drop support of v1 here (at which point people will have upgraded anyway) and then the internals of this module will be clean again.

[dehmer]

I wasn't even aware that memory-level and classic-level still have to adopt the promise API. Since I have a second AbstractLevelDOWN implementation which communicates between Electron renderer and main process through IPC, I will defer DOWN/UP architecture migration until all modules are in sync with abstract-level v2.

Thank you very much for the smooth collaboration and clarification. Godspeed for you guys! Cheers!