Writing a test today for `repliction key` s for a sql based Meltano #singer-tap-development

Writing a test today for `repliction_key`'s for a ...

visch

12/14/2022, 2:38 AM

Writing a test today for `repliction_key`'s for a sql based target based on the sdk. There's some behavior I didn't realize Meltano is doing in the catalog with metadata being set. 🧵 for more details so I don't spam the channel

visch

12/14/2022, 2:42 AM

meltano.yml

Copy code

version: 1
send_anonymous_usage_stats: true
project_id: "tap-postgres"
plugins:
  extractors:
  - name: "tap-postgres"
    namespace: "tap_postgres"
    pip_url: -e .
    capabilities:
    - state
    - catalog
    - discover
    config:
      sqlalchemy_url: "<postgresql://postgres:postgres@localhost:5432/postgres>"
    settings:
    - name: sqlalchemy_url
      kind: password
    select:
      - "public-test_replication_key.*"
    metadata:
      public-test_replication_key:
        replication-key: "updated_at"
        replication-method: "INCREMENTAL"
  loaders:
  - name: target-jsonl
    variant: andyh1203
    pip_url: target-jsonl

Outputs a catalog with

meltano invoke --dump=catalog tap-postgres > catalog.json

Puts

Copy code

replication-key: "updated_at"
        replication-method: "INCREMENTAL"

In the

stream

public-test_replication_key

's breadcrumb=[],

metadata

object , and it also adds a key to the top level object of

stream

"replication_key":"updated_at"

(I didn't instruct this to happen which throws me a bit). Without that

replication_key

at the top level the tap doesn't' realize it should be doing an incremental sync which is odd to me. Maybe this is a bug with the SDK? Also seems interesting that meltano adds this key to the catalog? Maybe Meltano is on purpose and the sdk isn't? Maybe I'm mistaken in some way PR Incoming with tap-postgres which should show this in a little bit of an easier form to read

visch

12/14/2022, 2:59 AM

https://github.com/MeltanoLabs/tap-postgres/pull/10/files#diff-8c860412f8d7721e24d8773bc60c85f4c19fbcf4c4767c659acf392b53a07926R75 Shows what is unexpected to me

aaronsteers

12/14/2022, 6:00 PM

Thanks for raising. Recapping from office hours... On first glance, I think we should probably get this from either the stream definition or the metadata

[]

breadcrumb. I'd have to look a bit deeper into the spec to confirm. cc @edgar_ramirez_mondragon

edgar_ramirez_mondragon

12/14/2022, 6:29 PM

Yeah, I think Meltano is playing safe and putting it in both locations since the catalog entry schema supports them: https://github.com/singer-io/singer-python/blob/2c053f4c5f468235bba85a203a252438b1b3c704/singer/catalog.py#L21-L32 https://github.com/meltano/sdk/blob/ee570e0068980613731c71ff6188da4e3b1941e2/singer_sdk/connectors/sql.py#L409-L414

visch

12/14/2022, 9:11 PM

Yes, and it looks like the Singer SDK requires you to set it at the stream level as well. Hmm maybe as @pat_nadolny pointed at maybe we should at this to the hub docs for the singer spec. There's a really good section there for replication-key and this portion isn't there

visch

12/14/2022, 9:11 PM

Is it a bug that the SDK requires it to be at the stream level as well? Or do we want to just leave it for now?

aaronsteers

12/14/2022, 9:16 PM

Source docs seems to confirm that the metadata level should be the authoritative. https://github.com/singer-io/getting-started/blob/ca5c56f16e67e0c2da49e9bb94d6f52fb19c9de6/docs/DISCOVERY_MODE.md#example-2

aaronsteers

12/14/2022, 9:16 PM

I'd suggest we coalesce from one to the other in the SDK when parsing - so one or the other would be sufficient.

aaronsteers

12/14/2022, 9:16 PM

@edgar_ramirez_mondragon - what do you think?

edgar_ramirez_mondragon

12/14/2022, 9:38 PM

I’d suggest we coalesce from one to the other in the SDK when parsing - so one or the other would be sufficient.

I agree. Although after diving a bit I can’t tell where in the SDK that’s currently being read from the catalog entry’s top-level 🤔

Open in Slack

Previous Next