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

[7/n][dagster-fivetran] Implement load_fivetran_asset_specs #25808

Merged
merged 5 commits into from
Nov 13, 2024

Conversation

maximearmstrong
Copy link
Contributor

@maximearmstrong maximearmstrong commented Nov 8, 2024

Summary & Motivation

This PR implements the load_fivetran_asset_specs function:

from dagster_fivetran import FivetranWorkspace, load_fivetran_asset_specs

import dagster as dg

# Connect to Fivetran using the credentials
fivetran_workspace = FivetranWorkspace(
    account_id=dg.EnvVar("FIVETRAN_ACCOUNT_ID"),
    api_key=dg.EnvVar("FIVETRAN_API_KEY"),
    api_secret=dg.EnvVar("FIVETRAN_API_SECRET"),
)

fivetran_specs = load_fivetran_asset_specs(fivetran_workspace)
defs = dg.Definitions(assets=[*fivetran_specs], resources={"fivetran": fivetran_workspace})

How I Tested These Changes

Additional unit test

Changelog

[dagster-fivetran] The load_fivetran_asset_specs function is added. It can be used with the FivetranWorkspace resource and DagsterFivetranTranslator translator to load your Fivetran connector tables as external assets in Dagster.

Copy link
Contributor Author

maximearmstrong commented Nov 8, 2024

to convert Fivetran content into AssetSpecs. Defaults to DagsterFivetranTranslator.

Returns:
List[AssetSpec]: The set of assets representing the Tableau content in the workspace.
Copy link

Choose a reason for hiding this comment

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

The docstring's return type description references Tableau content, but this function returns Fivetran assets. The return description should read: List[AssetSpec]: The set of assets representing the Fivetran content in the workspace.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

@maximearmstrong maximearmstrong marked this pull request as ready for review November 8, 2024 16:59
@maximearmstrong maximearmstrong self-assigned this Nov 8, 2024
@maximearmstrong maximearmstrong force-pushed the maxime/rework-fivetran-6 branch from 0e257e0 to e0fb1eb Compare November 8, 2024 17:00
@maximearmstrong maximearmstrong force-pushed the maxime/rework-fivetran-7 branch from 2000f24 to 5a21d96 Compare November 8, 2024 17:01
@maximearmstrong maximearmstrong force-pushed the maxime/rework-fivetran-6 branch from e0fb1eb to 77756ae Compare November 8, 2024 17:41
@maximearmstrong maximearmstrong force-pushed the maxime/rework-fivetran-7 branch from 5a21d96 to 1ae5e13 Compare November 8, 2024 17:41
@maximearmstrong maximearmstrong force-pushed the maxime/rework-fivetran-6 branch from 77756ae to de7c6f3 Compare November 8, 2024 19:45
@maximearmstrong maximearmstrong force-pushed the maxime/rework-fivetran-7 branch from 1ae5e13 to 7523673 Compare November 8, 2024 19:45
@maximearmstrong maximearmstrong force-pushed the maxime/rework-fivetran-6 branch from de7c6f3 to f12bbb7 Compare November 8, 2024 20:47
@maximearmstrong maximearmstrong force-pushed the maxime/rework-fivetran-7 branch from 7523673 to 84f264d Compare November 8, 2024 20:47
@maximearmstrong maximearmstrong force-pushed the maxime/rework-fivetran-6 branch from f12bbb7 to 0c52d6a Compare November 11, 2024 20:15
@maximearmstrong maximearmstrong force-pushed the maxime/rework-fivetran-7 branch from 84f264d to a2570c9 Compare November 11, 2024 20:15
@maximearmstrong maximearmstrong force-pushed the maxime/rework-fivetran-6 branch from 0c52d6a to 6126387 Compare November 11, 2024 22:21
@maximearmstrong maximearmstrong force-pushed the maxime/rework-fivetran-7 branch from a2570c9 to 8b78091 Compare November 11, 2024 22:21
to convert Fivetran content into AssetSpecs. Defaults to DagsterFivetranTranslator.

Returns:
List[AssetSpec]: The set of assets representing the Fivetran content in the workspace.
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add some examples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure - I added one in 14f49b7, more to come when I update the docs.

@maximearmstrong maximearmstrong force-pushed the maxime/rework-fivetran-7 branch from c8ec63b to 0923217 Compare November 12, 2024 22:56
Base automatically changed from maxime/rework-fivetran-6 to master November 13, 2024 12:41
@maximearmstrong maximearmstrong force-pushed the maxime/rework-fivetran-7 branch from 0923217 to 14f49b7 Compare November 13, 2024 12:50
@maximearmstrong maximearmstrong merged commit e500fa9 into master Nov 13, 2024
1 check passed
@maximearmstrong maximearmstrong deleted the maxime/rework-fivetran-7 branch November 13, 2024 13:08
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