Only write airbyte messages to duckdb on read

[clnoll]

This fixes an issue that @askarpets encountered during regression test runs for multiple connection IDs for source-pinterest (report here).

    | Traceback (most recent call last):
    |   File "/root/.cache/pypoetry/virtualenvs/live-tests-9TtSrW0h-py3.10/lib/python3.10/site-packages/asyncer/_main.py", line 164, in value_wrapper
    |     value = await partial_f()
    |   File "/app/src/live_tests/commons/connector_runner.py", line 171, in _run
    |     await execution_result.save_artifacts(self.output_dir, self.duckdb_path)
    |   File "/app/src/live_tests/commons/models.py", line 374, in save_artifacts
    |     self.save_airbyte_messages(output_dir, duckdb_path)
    |   File "/app/src/live_tests/commons/models.py", line 361, in save_airbyte_messages
    |     self.backend.write(self.airbyte_messages)
    |   File "/app/src/live_tests/commons/backends/duckdb_backend.py", line 64, in write
    |     duck_db_conn.sql(
    | duckdb.duckdb.NotImplementedException: Not implemented Error: Duplicate name "ad_group_id" in struct auto-detected in JSON, try ignore_errors=true

I was able to reproduce the issue locally.

I'm still not entirely sure why this is just coming up now for source-pinterest, but don't see a major advantage to storing the results for non-read commands in duckdb so am proposing this change. I haven't tried setting ignore_errors=true because it didn't feel like quite the right thing to do e.g. during read, but am open to other opinions.

Note: I found this issue that may be related, but rolling back the version didn't help.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	⬜️ Ignored (Inspect)	Visit Preview		May 24, 2024 1:25pm

[alafanechere]

@clnoll Duckdb was updated to a new minor version (0.10.3) in #38571 . Can you try downgrading it to 0.10.12 to check if it's a regression due to the update?

[clnoll]

Just added a note on that @alafanechere - I downgraded to 10.0.2 and it didn't fix the problem perhaps a bigger downgrade will work. Checking.

[alafanechere]

🙏 Can you please bump the package version?

[alafanechere]

Can we add a mapping like:

persist_command_to_duck_db = {
"read": True,
"write": True,
"spec": False,
"discover": False
}

Suggested change

	await execution_result.save_artifacts(self.output_dir, self.duckdb_path if airbyte_command == "read" else None)
	await execution_result.save_artifacts(self.output_dir, self.duckdb_path if persist_command_to_duck_db[airbyte_command] else None)

With a comment saying why we disable persistance on other commands?

[clnoll]

@alafanechere downgrading to 0.10.1 fixed the issue so I think that's the way to go here. Sorry for the back and forth.

[alafanechere]

Cool! Definitely a preferable fix!

[natikgadzhi]

Weird that a minor version bump on duckdb causes problems for us. What’s the plan long-term? Also, Augustin, why introduce a mapping when we only need to check for one specific operation?