pelias-deprecated / admin-lookup

A fast, local, streaming Quattroshapes administrative hierarchy lookup.
1 stars 0 forks source link

module.exports.stream interface #6

Closed missinglink closed 9 years ago

missinglink commented 9 years ago

the current module.exports.stream() interface makes it difficult to drop in to existing streams code.

In order to use the stream you need to first initialize the data store, the current callback interface means as a consumer I must wrap all my other code in the init callback.

In a worst case scenario it turns this code:

stream1.pipe( stream2 ).pipe( stream3 );

... in to this:

setupStream1( function( stream1 ){
  setupStream2( function( stream2 ){
    setupStream3( function( stream3 ){
      stream1.pipe( stream2 ).pipe( stream3 );
    });
  });
});

It would probably be better to return a streaming interface synchronously that simply blocks data flow until the data store is ready in order to maintain the flat, clean and easy to integrate streams interface.

Thoughts @sevko ?

sevko commented 9 years ago

There's no good reason to load the hierarchy asynchronously (since it only happens once as an initialization step), but I made it asynchronous because the shapefile package doesn't have a sync interface. It should only ever result in one level of callbacks, like so:

peliasAdminLookup.stream(function(lookup){
    dataPipeline.pipe(lookup).pipe(elasticsearch);
});

Regardless, it's still inconvenient, so I'll see if there's any other way of forcing synchronous execution.

sevko commented 9 years ago

How were you thinking the stream could block until everything's loaded?

missinglink commented 9 years ago

It would simply not call next() until the datasource was loaded.

sevko commented 9 years ago

Landed.