dynamodb icon indicating copy to clipboard operation
dynamodb copied to clipboard

scan and query always return AsyncIterators

Open hayd opened this issue 5 years ago • 4 comments

Fixes #4

@chiefbiiko This also makes the DynamoDBClient interface more explicit... This would be a breaking change, but IMO it's a more user-friendly API.


If you wanted convenience functions for scan+query that return Promise<Doc> we can special case i.e.

  dyno.queryOnce = async (params: Doc, options?: Doc): Promise<Doc> => {
   // TODO handle options === undefined
    options.iteratePages = false;
    for await (page of dyno.query(params, options)) {
      return page
    }
  }

  dyno.scanOnce = async (params: Doc, options?: Doc): Promise<Doc> => {
    options.iteratePages = false;
    for await (page of dyno.scan(params, options)) {
      return page
    }
  }

(save for a future PR - naming is discussion worthy).

hayd avatar Mar 03 '20 03:03 hayd

@hayd thanks for ur input, let me try to paper my thoughts: i want dynamodb to reflect the aws api amap - ideally no additional brain-drain layer

my concern regarding always-ait is that counting like

const { Count } = await dyno.scan({ TableName: "Tisch", Select: "COUNT" });

wouldnt work - would have to be

const { Count } = await dyno.scan({ TableName: "Tisch", Select: "COUNT" }, { iteratePages: false });

too unintuitive solution could be to point users to dyno.scanOnce or intro dyno.count but then thats dominos and bloat

also the scan op is expensive and sth to avoid by db design i dont necessarily want to "optimize" this part of the client with custom logic

but need some more thought on this..

chiefbiiko avatar Mar 03 '20 08:03 chiefbiiko

also the scan op is expensive and sth to avoid by db design

Whilst it is expensive it's often the case you need more that one page worth. 🤷‍♂

It would be:

const { Count } = await dyno.scan({ TableName: "Tisch", Select: "COUNT" }, { iteratePages: false });
// wouldn't work either, it would be:
const { Count } = await dyno.scanOnce({ TableName: "Tisch", Select: "COUNT" })

I don't think it's dominoes as these are the only two ops that have pagification.

You could also do it the other way scanMany or scanIter (or something). At the moment using it as asyncIterable is the more painful thing to use, so if you think scanIter is used less often maybe it makes sense to have it being the different name.

hayd avatar Mar 03 '20 09:03 hayd

leanin towards preserving scan but exposing scanIter to get an ait with guarantee

chiefbiiko avatar Mar 03 '20 13:03 chiefbiiko

Is a trick to define a new class that is both awaitable and async iterable?

i.e. await scan() returns :Doc and for await (const page:Doc of scan()) {} also works.

hayd avatar Mar 03 '20 19:03 hayd