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

replace parallel-transform with pipeline-pipe #321

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tadjik1
Copy link
Contributor

@tadjik1 tadjik1 commented Jun 30, 2024

👋 I did some awesome work for the Pelias project and would love for everyone to have a look at it and provide feedback.


Here's the reason for this change 🚀

Hey, I've noticed that the adminLookup stream doesn't end with the end of pbf parser and found an [https://github.com/mafintosh/parallel-transform/issues/4](untucked issue in the parallel-transform module). Someone came up with the solution by creating new module pipeline-pipe with slightly different syntax, but solving the same problem.
I've update dependency and tested with my local setup, I hope this would be helpful for you as well.


Here's what actually got changed 👏

Replace parallel-transform dependency with pipeline-pipe and converted callback-based transform function inside lookupStream with promise-based.


Here's how others can test the changes 👀

Now, you can handle end of the stream in pelias-openstreetmap module:

// default import pipeline
streams.import = function () {
  streams
    .pbfParser()
    .pipe(streams.docConstructor())
    .pipe(streams.addressesWithoutStreet())
    .pipe(streams.tagMapper())
    .pipe(streams.addressExtractor())
    .pipe(streams.blacklistStream())
    .pipe(streams.categoryMapper(categoryDefaults))
    .pipe(streams.addendumMapper())
    .pipe(streams.popularityMapper())
    .pipe(streams.adminLookup())
    .on('end', () => console.log('ended'))
};

In my case I'm not using Elasticsearch (and pelias-dbclient) and import data into different source after extraction/mapping.

@missinglink
Copy link
Member

Hi @tadjik1 thanks for the PR.

I'm trying to understand better, the issue you linked seems to suggest that the end event is emitted correctly at the end, but the finish event is emitted too early.

In your example you have .on('end', () => console.log('ended')), didn't this always work?
Is the problem with finish or end?

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.

2 participants