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

Added support for aliased models and source blocks #17

Merged
merged 7 commits into from
May 26, 2021

Conversation

z3z1ma
Copy link
Contributor

@z3z1ma z3z1ma commented May 10, 2021

Added alias support in model parsing. Also added searching for "sources" yml blocks pushing table/column documentation to metabase (defining sources is recommended by dbt best practices and often done in tandem with creating staging models). Allows using "metabase.fk_ref" in sources or models .yml in order explicitly define a metabase relationship to a table which allows us to propagate the FK relationship properly to Metabase regardless of aliasing. metabase.fk_ref is looked for first in meta before falling back to default parser.

Added alias support in model parsing. Also added searching for "sources" yml blocks pushing table/column documentation to metabase (defining sources is recommended by dbt best practices and often done in tandem with creating staging models). Allows using "metabase.fk_ref" in sources or models .yml in order explicitly define a metabase relationship to a table which allows us to propagate the FK relationship properly to Metabase regardless of aliasing. metabase.fk_ref is looked for first in meta before falling back to default parser.
Copy link
Owner

@gouline gouline left a comment

Choose a reason for hiding this comment

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

The code base is formatted with Black (https://github.com/psf/black), please format accordingly.

dbtmetabase/dbt.py Outdated Show resolved Hide resolved
dbtmetabase/dbt.py Show resolved Hide resolved
dbtmetabase/dbt.py Outdated Show resolved Hide resolved
Replace print with log and ensure multiple sources in a single YML can be looped through- also formatted according to black
replace single quoting with double to conform to style guideline
useful when project structure is defined and ymls created but not yet populated
@z3z1ma
Copy link
Contributor Author

z3z1ma commented May 25, 2021

@gouline

Changes requested are complete. I'll probably contribute more in the near future considering the successful adoption of dbt I worked with my company to achieve and our relatively high usage of metabase.

dbtmetabase/dbt.py Outdated Show resolved Hide resolved
with open(path, "r") as stream:
schema = yaml.safe_load(stream)
if schema is None:
logging.warn(f"SKIPPING EMPTY/INVALID YML: {path}")
Copy link
Owner

Choose a reason for hiding this comment

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

Please don't use all-caps in logs, it's inconsistent with other logs. Also no need for f-strings in logging statements, it takes arguments:

logging.warn("Skipping empty or invalid YAML: %s", path)

dbtmetabase/dbt.py Outdated Show resolved Hide resolved
replaced last prints with info level logs and made invalid yml logger msg match rest
Re-added processing model logger info msg
updated log to match recommendation
@z3z1ma
Copy link
Contributor Author

z3z1ma commented May 26, 2021

@gouline Changes complete as recommended. 👍

Copy link
Owner

@gouline gouline left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for all the commits!

@gouline gouline merged commit fa115fb into gouline:master May 26, 2021
@gouline gouline added this to the v0.8.x milestone Jul 28, 2021
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