osmlab / to-fix-backend

The to-fix server
BSD 3-Clause "New" or "Revised" License
15 stars 13 forks source link

[critical] unregistered callback #109

Closed geohacker closed 7 years ago

geohacker commented 7 years ago

We noticed that the backend seems to eat up geojsons with no feedback, and arbitrarily success or failure errors. After a bit of digging @batpad discovered these lines: https://github.com/osmlab/to-fix-backend/blob/master/src/controllers/ControllerTasks.js#L143, that invokes the import script but doesn't wait for the callback of the import script to register.

The import script is doing everything asynchronously and this means that the main process isn't waiting for the child to complete it's operations.

@Rub21 - can you tell us why this isn't a node module with a cli instead?

cc @maning @ajithranka

Rub21 commented 7 years ago

@Rub21 - can you tell us why this isn't a node module with a cli instead?

I am using this, because when we upload a huge GEOjson, the backend takes a lot time to process the features and upload the items to Elastic Search.

If we don't do this 👇
https://github.com/osmlab/to-fix-backend/blob/master/src/controllers/ControllerTasks.js#L143 the backend will response a 408 Request Timeout to the client,which os not good.

Also, when we create the a task, 👇

tfcli dev create --name 'test test' --description 'abc' --comment 'abc' --file test.geojson

After finishing the file upload, the backend will return really fast to the client 👇

CREATE TASK :http://localhost:8000/tasks
{"idtask":"testtest","isCompleted":false,"isAllItemsLoad":false,"iduser":"510836","status":true,"value":{"name":"test test","description":"abc","updated":1503926641,"changesetComment":"abc","stats":[{"edit":0,"fixed":0,"noterror":0,"skip":0,"type":"v1","items":0,"date":1503926641}]}}
mbtiles $ 

This item is important : "isAllItemsLoad":false it said the items isn't uploaded yet to Elastic Search, and when all items are uploaded it will change to true. here 👉 https://github.com/osmlab/to-fix-backend/blob/master/src/utils/import/index.js#L106,

geohacker commented 7 years ago

Right. Thank you for explaining @Rub21. We'd want to figure out a different approach to upload large GeoJSONs. Let's revisit.