apify / apify-storage-local-js

Local emulation of the apify-client NPM package, which enables local use of Apify SDK.
3 stars 4 forks source link

feat: RequestQueue v2 locking #59

Closed vladfrangu closed 1 year ago

barjin commented 1 year ago

Just checked the tests, and they seem fine to me :)

Some tiny nits:

B4nan commented 1 year ago

I'm just (very slightly) confused about the prepared statements management - can we have a Record<statementName, Statement | null> for them?

Same here, I was also a bit confused by that, but saw that it was done this way before... I'd argue we don't even need to go through the prepared statements for every single query, it makes sense only for those that will be repeated very often. Either way, this deserves some abstraction so we don't need to handle it again and again for every single query.

vladfrangu commented 1 year ago
  • And (on a very similar note), how about wrapping this._updateOrderNo.run({...}) in this.updateOrderNo({...}), which would basically be the _initUpdateOrderNo and this._updateOrderNo.run({...}) together? It might make the logic a bit clear (and we could ditch the preemptive calls to _initUpdateOrderNo in every method).

Good point, will change