Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: Slightly refactor fixed config values into tests/settings.py #312

Merged
merged 2 commits into from
Dec 12, 2023

Conversation

amotl
Copy link
Contributor

@amotl amotl commented Dec 11, 2023

Dear Derek and Edgar,

first things first: Thank you so much for your excellent work on MeltanoLabs' {tap,target}-postgres. We have been able to build upon them at 12 without much ado.

We fear we can't contribute anything significant back for the time being, other than cosmetic changes, if you like them. In this spirit, this patch makes a humble start. It was derived from crate-workbench/meltano-tap-cratedb#1, and pulls the database-specific configuration values into a tests/settings.py file, to make it easier to adjust them.

With kind regards,
Andreas.

Footnotes

  1. https://github.com/crate-workbench/meltano-tap-cratedb

  2. https://github.com/crate-workbench/meltano-target-cratedb

This intends to make it easier to derive the code into downstream test
cases, for databases which are largely compatible with PostgreSQL.
Copy link
Member

@edgarrmondragon edgarrmondragon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much cleaner. Thanks @amotl!

@edgarrmondragon edgarrmondragon merged commit 68b6644 into MeltanoLabs:main Dec 12, 2023
5 of 6 checks passed
@amotl
Copy link
Contributor Author

amotl commented Dec 12, 2023

That was quick. Thanks for merging!

@visch
Copy link
Member

visch commented Dec 15, 2023

@amotl Thank you very much! We love them, the cleaner the better :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants