Fix: Add schema in Postgres Cache to swapping method

airbytehq / PyAirbyte

PyAirbyte brings the power of Airbyte to every Python developer.

https://docs.airbyte.com/pyairbyte

Other

178 stars 20 forks source link

Fix: Add schema in Postgres Cache to swapping method #149

Closed tinomerl closed 3 months ago

tinomerl commented 3 months ago

Fixes #148

I dug a bit around and found out, that the Problem was, that the schema name was missing in the final swaping. I just added the method used at other places to add the schema.

I separated the statements in several lines so that i won't go over the defined Line Limit from 100.

Tests were all successful except for the integration Tests of some caches, since i don't have access to them.

Also tested the fix manuallay with the pokeapi source and the linkedinpages source mentioned in #148. Everything worked and data was in the final tables.

aaronsteers commented 3 months ago

/test-pr

Full test output (internal link): https://github.com/airbytehq/PyAirbyte/actions/runs/8470339797/job/23207828832

aaronsteers commented 3 months ago

@tinomerl - This is great. Thank you!

I found a small typo and just applied the fix (adding back the space between "RENAME" and "TO").

Re-running our test suite here: https://github.com/airbytehq/PyAirbyte/actions/runs/8470606404

I'm not sure if you'll be able to see summary status or if that link will work at all. But I'll watch it and report back. Once tests pass, we should be clear to merge.

I'm also using this PR as a test for ability to run CI on forks. I merged in a fix from main that should allow them to be run correctly. I can see now that the "Fast" and "No-Creds" versions of the Pytest CI workflows are working correctly and getting the green checkmark. ✅ 🙌

tinomerl commented 3 months ago

@aaronsteers - thanks! I'm happy to help.

Oh darn. I saw the typo. Should have caught that one myself by looking at the very first runs. 🙈

The link for the rerun works for me and I can see the results.

Great. That means it's easier and faster for you guys to check PRs from forks right?

aaronsteers commented 3 months ago

@tinomerl - Just for my info - can you confirm if you can see results only, or also the detailed logs?

aaronsteers commented 3 months ago

Great. That means it's easier and faster for you guys to check PRs from forks right?

Exactly that. 😄

aaronsteers commented 3 months ago

Will merge shortly.

Tests are green here in the PR (auto-skipping tests that require creds), and tests are also green on the manual workflow here (which uses our creds to do the full suite of tests).

tinomerl commented 3 months ago

@tinomerl - Just for my info - can you confirm if you can see results only, or also the detailed logs?

@aaronsteers I can also see the logs of every step when clicking it. But only after it's finished, if this makes any difference.