datahub icon indicating copy to clipboard operation
datahub copied to clipboard

feat(ingest): add platform instance to tableau

Open alaponin opened this issue 3 years ago • 2 comments

This feature adds an optional platform instance to the Tableau connector to capture the uniqueness of the tableau server and site.

For testing, a new integration test was created.

Checklist

  • [ ] The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • [ ] Links to related issues (if applicable)
  • [x] Tests for the changes have been added/updated (if applicable)
  • [x] Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • [ ] For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

alaponin avatar Sep 19 '22 15:09 alaponin

Unit Test Results (metadata ingestion)

       8 files  ±0         8 suites  ±0   58m 46s :stopwatch: - 1m 20s    738 tests +1     736 :heavy_check_mark: +1  2 :zzz: ±0  0 :x: ±0  1 478 runs  +2  1 474 :heavy_check_mark: +2  4 :zzz: ±0  0 :x: ±0 

Results for commit 29d42c87. ± Comparison against base commit 521e61d3.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Sep 19 '22 16:09 github-actions[bot]

Unit Test Results (build & test)

597 tests  ±0   593 :heavy_check_mark: ±0   12m 5s :stopwatch: -1s 147 suites ±0       4 :zzz: ±0  147 files   ±0       0 :x: ±0 

Results for commit 29d42c87. ± Comparison against base commit 521e61d3.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Sep 19 '22 17:09 github-actions[bot]

LGTM!

Looks like there's an error + conflict in the tests - we recently made some big simplifications to the tests so you might need to regenerate the golden file.

Done, looks like there's a need for a workflow approval.

alaponin avatar Oct 11 '22 10:10 alaponin

@alaponin thanks for fixing the merge conflicts! I've re-triggered the CI so hopefully we can merge this today :)

hsheth2 avatar Oct 13 '22 18:10 hsheth2

@alaponin thanks for fixing the merge conflicts! I've re-triggered the CI so hopefully we can merge this today :)

Fixed some linting issues. Hopefully the checks will pass now.

alaponin avatar Oct 13 '22 19:10 alaponin