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

Feature/dagster elx asset loading #24

Merged
merged 13 commits into from
Nov 21, 2023

Conversation

BernardWez
Copy link
Member

No description provided.

Copy link
Collaborator

@JulesHuisman JulesHuisman left a comment

Choose a reason for hiding this comment

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

Zou kijken of nieuwe tests kunnen gedaan worden door de huidige fixtures. Over het algemeen maak je niet voor alles nieuwe fixtures. Je kan bijvoorbeeld ook een specifieke situatie maken in de test function zelf als het echt nodig is.

tests/fixtures/tap.py Outdated Show resolved Hide resolved
tests/fixtures/runner.py Outdated Show resolved Hide resolved
@BernardWez
Copy link
Member Author

Ja, vond het zelf ook al niet helemaal mooi. Heb die custom incremental tap die ik had gemaakt nu als de basis genomen voor de fixtures. Die heeft default 2 streams: 1 incremental en 1 full table. Moest idd dan wel ff testen hier en daar wat fixen.

Copy link
Collaborator

@JulesHuisman JulesHuisman left a comment

Choose a reason for hiding this comment

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

Nice, dat ziet er een stuk cleaner uit!

@BernardWez BernardWez merged commit eaec5ac into master Nov 21, 2023
4 checks passed
@BernardWez BernardWez deleted the feature/dagster-elx-asset-loading branch December 1, 2023 08:29
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.

2 participants