pg-mvt: Implement timeout in getTile #661
Conversation
| // Init DB and get layer column names. | ||
| initDB (callback) { | ||
| try { | ||
| this.psql = new PSQL(this.dbParams, this.dbPoolParams); |
There was a problem hiding this comment.
I'd move the creation of the connection to the constructor and rename this function (e.g: getAllLayerColumns)
| debug("Error running pg-mvt query " + query + ": " + err); | ||
| if (err.message) { | ||
| err.message = "PgMvtRenderer: " + err.message; | ||
| const pr_getTile = new Promise((resolve, reject) => { |
There was a problem hiding this comment.
What about extracting this promise to a new method of the class.
There was a problem hiding this comment.
That was my initial idea, then I saw that I needed the query string to be able to cancel and decide to merge it all together, but as I can see now I can have a function generating the promise that receives the query as a parameter.
| /// @param z tile zoom | ||
| /// callback: will be called when done using nodejs protocol (err, data) | ||
| getTile (z, x, y, callback) { | ||
| return this._getTileTimeout(z, x, y, this.render_limit, callback); |
There was a problem hiding this comment.
Why not move the content of_getTileTimeout to getTile?
I'll review it. I think it comes from switching languages and styles between projects 😅 |
|
Comments addressed. I had to add an extra commit to test this locally (Postgis 3.0). |
| this.psql = new PSQL(this.dbParams, this.dbPoolParams); | ||
| } | ||
|
|
||
| // Init DB and get layer column names. |
There was a problem hiding this comment.
Careful! Update the comment!
Since you need to open a new connection to close a running query I had to move the connection parameters to the renderer, so I decided to move
getLayerColumnstoo and add ainitDBfunction to avoid handling promises in the constructor.To get the limits inside this renderer I've modified the factory to add
mapnik.mapnik.limitstomvt, which is not pretty but needed for compatibility.Closes #577
Closes #624