search

singer-io / tap-bing-ads

A Singer.io tap for extracting data from the Bing Ads API
GNU Affero General Public License v3.0
12 stars 28 forks source link

Impossible to set PK on Reports #45

Open judahrand opened 4 years ago

judahrand commented 4 years ago

This is an issue which I've noticed across a few taps but first here so I'll raise the issue here and take it from there.

In my use case I am pulling static reports from various taps and using a target to store them in a database. Because the reports are static I know what the primary key (PK) should be.

It would be good to be able to set the PK in a similar fashion as was just merged into tap-adwords (https://github.com/singer-io/tap-adwords/pull/58)

This would involve changing this line to:

    singer.write_schema(report_stream.stream,
                                     report_schema,
                                     metadata.get(stream_metadata, (), 'tap-bing-ads.report-key-properties') or [])
   )

This change would, of course, need the relevant documentation additions.

Please do let me know if such a PR would be approved and I'll put it in ASAP.

judahrand commented 4 years ago

I've realised there is an issue with this approach... This tap seems not really support the current Singer Spec as the generated metadata does not seem to include an empty breadcrumb entry. This would also need to be added. And while one was at it they could make it use the current select criteria from the metadata for the stream too.

judahrand commented 4 years ago

Can someone look at this?

ilkkapeltola commented 2 years ago

Two years later, this still bugs me out. If I take this forward to a target that doesn't have any information about primary keys, but needs to, this fails. Can you please fix this?