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

Exposure Parsing #38

Merged
merged 129 commits into from
Aug 1, 2021
Merged

Exposure Parsing #38

merged 129 commits into from
Aug 1, 2021

Conversation

z3z1ma
Copy link
Contributor

@z3z1ma z3z1ma commented Jul 20, 2021

Description

Reposting because some branch renaming fudged up the PR. This is still ready for review.

Parsing exposures from Metabase automatically in relation to a dbt project. This function is huge for understanding exposures within your data model. Close the loop between model composition and tangible BI exposures of those models creating an extremely valuable company asset in the dbt docs. In the nature of this project, the aim is we can use almost the same invocation parameters users set up in CI / CD for dbt-metabase export as they can with dbt-metabase exposures. This lets you run the invocation in production to essentially keep you docs in full sync with BI.

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How Has This Been Tested?

Open to ideas but ultimately I think making a mock api from metabase running on top of a database that has been seeded and ran through with the dbt jaffle shop project would be most robust. Just a lot of work.

  • CI test for exposure documentation

Test Configuration:

  • Python version: 3.8

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

References #22

falador_wiz1 and others added 30 commits June 12, 2021 14:06
…ersion of dbt-metabase having improved yaml parsing. large formatting update to conform to black.
…special or semantic until ready to deprecate
…e properly used if found in metabase api response.
… without depends on/test_metadata dont throw
…xpected input format to be defined in readme. otherwise automatic resolution of target field using relation test will prepend target run schema which should be fine in 95% of use cases. cases outside that can use manifest.json parsing or set fk_ref in yml.
…d ensured support for schema agnostic fk targets (schema resolved from manifest.json)
…sync will now only fail hard if timeout is explicit, otherwise default behaviour if --sync is true is to attempt sync for 30 seconds and proceed with aligning what can be aligned successfully. more formatting and a few comments for clarity of intent. also added option to pass custom cert bundle to verify.
…usly with seemless function alongside primary artifact parser (manifest.json).
…f regex to permit catching last arg of either ref or source always being the target table. if pointing to an alias, we are collecting aliases during yml parsing to be passed to metabased client and translated to metabase table names as needed. this functionality should be unnoticed by the user but provide more resiliency as well as more user friendly outcome whilst still being very specific in our logging.
…on path strings allowing relative paths for --dbt_path or --dbt_manifest_path simplified
…ntees us `schema.table` format. This allows us to guarantee the incorrectly formatted ref (which should be `schema.table`) is originating from yml. Log the warning and infer correct schema for our users using target schema which covers the 90% use case.
…nit__ to nonemptystr class, cleaned some logging calls to use lazy interpolation
@z3z1ma
Copy link
Contributor Author

z3z1ma commented Jul 23, 2021

@erika-e

Both schema and schema_excludes only restrict the scope of the dbt parser.
the same follows for the includes and excludes arguments (so for example with --includes my_model appended to your typical invocation, you can propogate docs / extract exposures to an output file for a single model.

Everything is pulled from Metabase and checked against the subset of dbt models for table (model) name matches from dbt <-> Metabase.

test_table is a model in your manifest/dbt project?

@gouline gouline mentioned this pull request Jul 24, 2021
@erika-e
Copy link
Contributor

erika-e commented Jul 24, 2021

@erika-e

Both schema and schema_excludes only restrict the scope of the dbt parser.
the same follows for the includes and excludes arguments (so for example with --includes my_model appended to your typical invocation, you can propogate docs / extract exposures to an output file for a single model.

Everything is pulled from Metabase and checked against the subset of dbt models for table (model) name matches from dbt <-> Metabase.

test_table is a model in your manifest/dbt project?

Yes, I changed the name from the real one to the more generic test_table but it's definitely there. I checked the manifest to be sure and it's there with the same name. Totally possible it's user error still. 😅 I'll do some more testing and debugging and see if I can pin the problem down.

@z3z1ma
Copy link
Contributor Author

z3z1ma commented Jul 27, 2021

@gouline
All changes based on excellent user usage review as well as my own usage has been implemented. Now I need to integrate any readme updates to point out the renamed flags, but functionally, I dont think there is much more to do. The unit tests are very good in displaying the function on a full mock api with dbt model project seeded generating the exact expectation of auto generated exposure documentation.

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.

Looking good, only a few small issues left. Almost ready for merging.

dbtmetabase/models/config.py Outdated Show resolved Hide resolved
dbtmetabase/parsers/dbt_folder.py Outdated Show resolved Hide resolved
dbtmetabase/parsers/dbt_folder.py Outdated Show resolved Hide resolved
@gouline gouline added this to the v0.8.x milestone Jul 28, 2021
@z3z1ma
Copy link
Contributor Author

z3z1ma commented Jul 30, 2021

@gouline

All requested changes complete. Plus a little cherry in ensuring our config objects input args match our cli flag args verbatim for additional ease of use between the two methods of utilizing the package.

@z3z1ma z3z1ma requested a review from gouline July 30, 2021 16:05
README.rst Outdated Show resolved Hide resolved
dbtmetabase/metabase.py Outdated Show resolved Hide resolved
dbtmetabase/metabase.py Show resolved Hide resolved
dbtmetabase/metabase.py Outdated Show resolved Hide resolved
dbtmetabase/metabase.py Show resolved Hide resolved
dbtmetabase/metabase.py Outdated Show resolved Hide resolved
@z3z1ma z3z1ma requested a review from gouline August 1, 2021 07:35
@z3z1ma
Copy link
Contributor Author

z3z1ma commented Aug 1, 2021

@gouline

All polished up. I think its as good as can be now (and super readable). Thanks for the review and suggestions.

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.

Awesome, mammoth effort! Thanks for persevering 🚀

@gouline gouline merged commit a1fa5a2 into gouline:master Aug 1, 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.

3 participants